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

Fix parallel build overflow for last bucket #7919

Closed
wants to merge 1 commit into from

Conversation

oerling
Copy link
Contributor

@oerling oerling commented Dec 7, 2023

A parallel build inserts one range of buckets per thread. If an insert does not fit in the last bucket in the range, it is added to overflows. The overflows are inserted sequentially at the end of the build. When inserting overflows, there are no partition bounds and as long as there is at least one free slot the insert cannot fail.

However, when inserting the overflows, the upper bound of the partition must be -1 to indicate no bounds. If it is sizeMask + 1 and the last bucket is full, the insert cannot wrap around to the first bucket like it should.

Fixes #3874

A parallel build inserts one range of buckets per thread. If an insert does not fit in the last bucket in the range, it is added to overflows. The overflows are inserted sequentially at the end of the build.
When inserting overflows, htere are no partition bounds and as long as there is at least one free slot the insert cannot fail.

However, when inserting the overflows, the upper bound of the
partition must be -1 to indicate no bounds. If it is sizeMask + 1 and
the last bucket is full, the insert cannot wrap around to the first
bucket like it should.
Copy link

netlify bot commented Dec 7, 2023

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 1195ef5
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/657246a174c7e800084c5d96

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 7, 2023
@oerling oerling requested a review from xiaoxmeng December 7, 2023 22:28
@facebook-github-bot
Copy link
Contributor

@oerling has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

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

@oerling Would you add a test?

@xiaoxmeng
Copy link
Contributor

@oerling #7925 has fixed this. Close this one. Thanks!

@xiaoxmeng xiaoxmeng closed this Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parallel hash build does not handle partition overflows
4 participants