Skip to content

feat: parallel preview processing#2569

Draft
valoq wants to merge 2 commits into
gokcehan:masterfrom
valoq:worker
Draft

feat: parallel preview processing#2569
valoq wants to merge 2 commits into
gokcehan:masterfrom
valoq:worker

Conversation

@valoq

@valoq valoq commented May 3, 2026

Copy link
Copy Markdown
Contributor

Addresses #2566

This PR is a very simple and small addition that speeds up preview generation (with or without preload) by using parallel processing of preview files. The number of workers is half the available cpu cores, which avoid a configuration setting and should provide a reasonable approach regardless of hardware.

@CatsDeservePets CatsDeservePets left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is just a draft but making preview processing non-deterministic is a bad idea. It introduces race conditions. Most obvious when drawing images to the terminal directly (required for unsupported protocols like kitty). Images get drawn in the wrong order and overlap each other.

Comment thread nav.go
Comment on lines +742 to +750
workers := max(2, runtime.NumCPU()/2)
for i := 0; i < workers; i++ {
go func() {
for w := range nav.workChan {
nav.preview(w.path, ui.wins[len(ui.wins)-1], w.mode)
}
}()
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quoting gopls:

for loop can be modernized using range over int [default]

@valoq

valoq commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

I know this is just a draft but making preview processing non-deterministic is a bad idea. It introduces race conditions. Most obvious when drawing images to the terminal directly (required for unsupported protocols like kitty). Images get drawn in the wrong order and overlap each other.

This should now be fixed by only using parallel workers for preload

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants