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

Switch from string keys to ReadOnlyMemory<byte> #1

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

Conversation

NickStrupat
Copy link

This way, any byte-representation can be used as a key.

@odinmillion
Copy link
Owner

Amazing PR! It is a good proposal to generalize library.

I am a little bit confused on this line: https://github.com/odinmillion/Phf.Net/pull/1/files#diff-c0abc29d441dd7573197a5233567ba9bR196
Maybe we have to pack bytes denser: 4 bytes per int32 instead of the 2 bytes.

@NickStrupat
Copy link
Author

Ah yes, I missed that. Should the 16 be 8? In both places (https://github.com/odinmillion/Phf.Net/pull/1/files#diff-c0abc29d441dd7573197a5233567ba9bR207)

@odinmillion
Copy link
Owner

It should be a little trickier) In case of C# strings we map each char to 2 bytes. Therefore we have only 2 possibilities: zero bytes remainder (str.Length % 2 == 0) and 2 bytes reminder (str.Length % 2 == 1). After switching to the bytes we have 4 possibilities for remainder length: 0, 1, 2, 3. Therefore we have to handle all that cases. Later I will try to implement this.

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.

2 participants