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

irmin: optimise V1.Hash.encode/decode #2028

Merged
merged 1 commit into from
Aug 9, 2022
Merged

Conversation

samoht
Copy link
Member

@samoht samoht commented Jul 30, 2022

Do not allocate intermediary strings to encode and decode V1 hashes.

@samoht samoht added the type/performance Rlated to performance label Jul 30, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 30, 2022

Codecov Report

Merging #2028 (93e7af1) into main (93e7af1) will not change coverage.
The diff coverage is n/a.

❗ Current head 93e7af1 differs from pull request most recent head 585956c. Consider uploading reports for the commit 585956c to get more accurate results

@@           Coverage Diff           @@
##             main    #2028   +/-   ##
=======================================
  Coverage   64.53%   64.53%           
=======================================
  Files         129      129           
  Lines       15561    15561           
=======================================
  Hits        10042    10042           
  Misses       5519     5519           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

encode_bin e f

let decode_bin buf pos_ref =
pos_ref := !pos_ref + 8;
Copy link
Member

Choose a reason for hiding this comment

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

Where does this 8 (and the one below) come from?

Copy link
Member Author

Choose a reason for hiding this comment

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

that's the size to encode in int64

Copy link
Member Author

Choose a reason for hiding this comment

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

@samoht samoht added the no-changelog-needed No changelog is needed here label Aug 4, 2022
Do not allocate intermediary strings to encode and decode V1 hashes.
@metanivek metanivek merged commit b5d5e3f into mirage:main Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog-needed No changelog is needed here type/performance Rlated to performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants