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

Avoid OOM when resizing very large dictionaries. #24

Merged
merged 1 commit into from
Jul 8, 2023
Merged

Conversation

VSadov
Copy link
Owner

@VSadov VSadov commented Jul 7, 2023

Minor fix.

There is a logic that prevents allocating too many candidate tables when resizing large dictionaries.

The logic had a flaw that would make it not work when the size is really really large. That could allow allocation of numerous huge candidates that would often result in OOM failure in a scenario which would otherwise be survivable.

// Size calculation: 2 words (K+V) per table entry, plus a handful. We
// guess at 32-bit pointers; 64-bit pointers screws up the size calc by
// 2x but does not screw up the heuristic very much.
int kBs4 = (((newsz << 1) + 4) << 3/*word to bytes*/) >> 12/*kBs4*/;
Copy link
Owner Author

@VSadov VSadov Jul 7, 2023

Choose a reason for hiding this comment

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

The bug was here. Left shifts could turn the large size estimate into 0, leading to allocating multiple candidates (since we assumed that would be cheap and fast).

The rest is just rationalizing and cleaning up this logic a bit.

Copy link
Owner Author

@VSadov VSadov Jul 7, 2023

Choose a reason for hiding this comment

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

This mostly matters in corner cases when dealing with abnormally huge dictionaries.

@VSadov VSadov merged commit bf998be into main Jul 8, 2023
@VSadov VSadov deleted the lrgAlloc branch July 8, 2023 14:59
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.

None yet

1 participant