Closed haberdashPI closed 12 months ago
Merging #29 (6b66e60) into main (e2b4f9a) will decrease coverage by
2.03%
. The diff coverage is95.86%
.:exclamation: Current head 6b66e60 differs from pull request most recent head 2c9126e. Consider uploading reports for the commit 2c9126e to get more accurate results
@@ Coverage Diff @@
## main #29 +/- ##
==========================================
- Coverage 97.26% 95.23% -2.03%
==========================================
Files 1 1
Lines 146 210 +64
==========================================
+ Hits 142 200 +58
- Misses 4 10 +6
Files | Coverage Δ | |
---|---|---|
src/StableHashTraits.jl | 95.23% <95.86%> (-2.03%) |
:arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
NOTE: I'd recommend reviewing this in something with smarter diffs than GitHub's online view (e.g. VSCode) as there are a number of lines with changed indentation and GitHub reads these as completely new lines where better diff views will show that mostly only the indentation has changed.
This refactors how the hash algorithm is used when computing
stable_hash
so that I can optimize performance more easily. Then it implements two wrappers that allow data to be stored in a buffer before the hash is computed:BufferedHash
: This actually stores data and "flushes" it when the buffer gets big enough by computing a hash on the buffer data.MarkerHash
: For buffering to be effective we need to avoid computing recursive hashes of every value. This wrapper represents nested calls tostable_hash
using hash delimeters that mark the start and end of a given object.MarkerHash
changes the hash values returned bystable_hash
, so I've implemented a newHashVersion{2}
; this new buffering implementation is only used when the root context (which is found by recurisvely callingparent_context
) has a hash version of 2. Thus,HashVersion{1}
and any contexts that have it as a parent, remain slow.Note that I plan to make a few more changes that will again change the hash value (#30), so this does not bump the version in
Project.toml
since there is a planned follow-up PR that will be merged before releasing a new version.Here are the benchmarks before, and after this PR is applied:
Before
After