Skip to content

Faster pattern picker popup #2264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

shewitt-au
Copy link
Contributor

@shewitt-au shewitt-au commented May 20, 2025

Problem description

I’ve found ImHex to be impressively zippy. I’ve no doubt that’s intentional. Why use the GPU to render the UI otherwise?

Opening the pattern file selection popup, however, takes ages (on my system at least). On a release build just over 7 seconds (much worse in debug builds, around 50 seconds!). Enough time for one to wonder if it’s working.

The reason is that ImHex has to preprocess every pattern file to extract the pattern's descriptions.
This PR uses a standalone parser designed just extract the description, and only loads the initial portion of the file when doing so, assuming the description pragma will be near the beginning.

Implementation description

Pretty much as described above. I won’t blame you if you object to the handwritten parser however. Noxiously fiddly problematic things. That said, some effort at least has been expended to match the behaviour of pattern_language’s parsing.

There's a magic number.

Even so, I suggest you try it out and see the difference. As more and more patterns are added the problem will only get worse and worse.

This is my second attempt at this PR. Rouge files made it into my last attempt. Still leanring how to use git properly. Sorry.

@shewitt-au shewitt-au marked this pull request as ready for review May 20, 2025 16:24
@shewitt-au
Copy link
Contributor Author

shewitt-au commented May 25, 2025

I've reopened this one. Turns out I can't live without it. So much faster. I apply it to every build I make for debugging when I have to open patterns and every release build for general use.

@WerWolv
Copy link
Owner

WerWolv commented May 25, 2025

Thanks for the PR!
I honestly wonder what makes it so slow for you. Even with every single pattern from the content store installed, it only takes about a second in a Debug build to process all those files. I'd generally like to avoid having multiple parsers for the same thing since it makes maintaining the whole system harder and requires multiple things to be kept in sync

Here's a video showcasing the speed on my machine:

Recording.2025-05-25.202745.mp4

@paxcut
Copy link
Collaborator

paxcut commented May 25, 2025

it is pretty slow for me too. it is not the auto loader it is when you click on import pattern on the menu

@shewitt-au
Copy link
Contributor Author

shewitt-au commented May 25, 2025

Import.pattern.mp4

I understand your reluctance to have parallel parsers. That said it’s only interested in #pragma directives. And it really is slow. So slow I initially though the app was broken. But we are loading it in different ways.

@shewitt-au
Copy link
Contributor Author

And the with the cutom parser

Faster.mp4

@WerWolv WerWolv closed this in 38ef005 May 25, 2025
@WerWolv
Copy link
Owner

WerWolv commented May 25, 2025

I did circumvent the preprocessor completely now and only run the lexer on the input files. For me that produces about the same speeds and doesn't require the reimplement a parser from scratch again. Could you try that latest commit and let me know if it's as fast for you too?

@shewitt-au
Copy link
Contributor Author

shewitt-au commented May 25, 2025

App hangs loading fonts on splash screen. I sould have, as you suggested, just pulled that one commit.

@shewitt-au
Copy link
Contributor Author

shewitt-au commented May 25, 2025

Ok. Killed the process and it started up the second time. It's slower than with the custom parser but still much faster than before both our changes. The code is a lot cleaner. I think it's a good compromise. On subsequent starts the app seems to start up quicker than before I pulled and compiled.

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.

3 participants