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

Generate file class dictionary after file classification for stability #944

Merged
merged 2 commits into from Nov 22, 2019

Conversation

@pmatilai
Copy link
Contributor

pmatilai commented Nov 21, 2019

Store the file type strings in the classifier, and generate the dictionary and its ids serially after the parallel section completes to ensure stable order. Besides making the classifying really run in parallel again, this also moves the pool- and file-counting related constraints out of the parallel section for theoretically better parallelization.

This is an alternative fix to #934, using omp order clause simply made the classification run serially.

pmatilai added 2 commits Nov 21, 2019
The order directive might be useful in some cases, but for our purposes
it very effectively serializes the whole classification operation.
Which means that we get the speed of serial classification with the
complexity of parallel execution, ugh. Revert, we need a better fix.

This reverts commit 3691d99.
Store the file type strings in the classifier, and generate the dictionary
and its ids serially after the parallel section completes to ensure
stable order. Besides making the classifying really run in parallel
again, this also moves the pool- and file-counting related constraints
out of the parallel section for theoretically better parallelization.

Fixes #934
@pmatilai

This comment has been minimized.

Copy link
Contributor Author

pmatilai commented Nov 21, 2019

@bmwiedemann if you can verify this works for you, I'd appreciate.

@pmatilai pmatilai added the REGRESSION label Nov 21, 2019
@bmwiedemann

This comment has been minimized.

Copy link
Contributor

bmwiedemann commented Nov 21, 2019

Tested it with our khmeros-fonts package build. Results are still reproducible.

@pmatilai

This comment has been minimized.

Copy link
Contributor Author

pmatilai commented Nov 21, 2019

Ack, thanks for verifying.

@pmatilai pmatilai merged commit b55fef0 into rpm-software-management:master Nov 22, 2019
3 checks passed
3 checks passed
LGTM analysis: Python No code changes detected
Details
LGTM analysis: C/C++ No new or fixed alerts
Details
semaphoreci The build passed on Semaphore.
Details
@pmatilai pmatilai deleted the pmatilai:fc-par-pr branch Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.