Use list of string lengths + list of strings instead of escaped null-terminated strings#11
Open
arai-a wants to merge 1 commit intobinast:masterfrom
Open
Use list of string lengths + list of strings instead of escaped null-terminated strings#11arai-a wants to merge 1 commit intobinast:masterfrom
arai-a wants to merge 1 commit intobinast:masterfrom
Conversation
…terminated strings
|
Also, I suspect that this will improve compression a little bit. |
Author
|
Checked the file size with facebook files in binjs-ref repository. also, checked another variant that sorts the string dictionary by length first. unfortunately this change mostly increases the file size. Apparently the length part isn't much compression-friendly compared to null-terminator.
|
|
Thank you for running this analysis @arai-a |
Author
|
there are 2 problems that this is supposed to solve:
the latter issue about "even if there's no escaped 0x01 or 0x00 in strings" can be solved by separating dictionary for strings that contains 0x01/0x00 and others. but having 2 dictionary structure just for that purpose doesn't sound reasonable for me. then, there is the following thing that in either way the decoder have to do:
So, in any way we have to iterate over the strings at least once (or twice if we decode), |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
changed String dictionary section to the following,
now we don't have to do escape, nor look for 0x01/0x00 while reading.