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

[Python, flexbuffers] key/string sharing feature not working with Python implementation #8283

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

guillaumeblanc
Copy link

Hi,

Python implementation fails to share keys and stings, resulting in bigger buffers than expected.

The reason is that the Pool implementation does a binary search on an unsorted array/pool. Hence, search fails (randomly), preventing sharing.

This PR contains:

  • Updated unit tests
  • Pool sharing algorithm fix
  • _ReadKey optimization, which appears to be critical now keys are shared, as otherwise processing a 20MB buffers takes hours instead of seconds.

Regards,
Guillaume

guillaumeblanc and others added 4 commits April 17, 2024 23:01
Binary search requires the pool to be sorted, which wasn't the case. Insertion now happens sorted, at the index returned by _LowerBound.
…e sorted.

The new implementation prevents from duplicating a big part of the buffer ([offset:]) to just access a key or a string. The assumption is that reusing keys and strings means [offset:] can become a bigger range (offset can now be at buffer begin).
@github-actions github-actions bot removed the c++ label Apr 18, 2024
@guillaumeblanc guillaumeblanc changed the title [Python] Fixes keys and strings sharing feature [Python, flexbuffers] Fixes keys and strings sharing feature Apr 19, 2024
@guillaumeblanc guillaumeblanc changed the title [Python, flexbuffers] Fixes keys and strings sharing feature [Python, flexbuffers] key/string sharing feature not working with Python implementation Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant