Skip to content
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

Optimized IsFileValid #65

Merged
merged 6 commits into from
May 25, 2024
Merged

Conversation

takahiro0327
Copy link
Contributor

Fixed IsFileValid was slow for large files over 200MB. Please review and merge.

400ms became about 80ms.
Converted the token search algorithm to BoyerMoore and to run on a thread pool.
When using NVMe, the major cost was token lookup on the CPU.

Copy link
Collaborator

@ManlyMarco ManlyMarco left a comment

Choose a reason for hiding this comment

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

It looks like for very large scenes there may be hundreds of threads created, which will be slower and could possibly cause issues by making thread pool fill up.

There's also a possible race condition where the last chunk completes before a previous chunk with the tag in it completes, causing a false negative.

Maybe it would be possible to use RunParallel? It would avoid the race condition and only use as many threads as necessary.

@takahiro0327
Copy link
Contributor Author

Thanks for review.

It looks like for very large scenes there may be hundreds of threads created, which will be slower and could possibly cause issues by making thread pool fill up.

The PoolAllocator was restricted by setting an upper limit on the number of instances.

There's also a possible race condition where the last chunk completes before a previous chunk with the tag in it completes, causing a false negative.

Sorry, I was an idiot. Fixed.

Maybe it would be possible to use RunParallel? It would avoid the race condition and only use as many threads as necessary.

It's in the form of making a list at the offset of the position to be read and executing it in parallel, right?

In many cases, it is faster to do nothing but sequential reads. This is especially true for HDDs with physical moving parts and slow SDDs.
It should be better to call fs.Read() sequentially in the main thread to get better speed in many environments.
It is not the fastest in certain environments, but considering the penalty in other environments, I think the current form is better.

Incidentally, in my environment, it appears that a maximum of 5 pools are used for a 200 MB scene.

Copy link
Collaborator

@ManlyMarco ManlyMarco left a comment

Choose a reason for hiding this comment

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

Makes sense. Thanks for the fixes, everything seems to be good now.

@ManlyMarco ManlyMarco merged commit c59bccc into IllusionMods:master May 25, 2024
@takahiro0327 takahiro0327 deleted the huge_scene2 branch June 2, 2024 12:09
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