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

Double id bits in ptr_uint64 and other clean-up #591

Merged
merged 12 commits into from
Dec 3, 2023

Conversation

SSoelvsten
Copy link
Owner

@SSoelvsten SSoelvsten commented Dec 3, 2023

Double the maximum width of bdd and zdd to 6 TiB at the cost of decreasing the maximum label by two. Furthermore, now that am looking back at this code from the prior refactor, I also clean up a few other things related to the pointer and add missing unit tests.

This way, we almost regain an entire bit for the level identifier, bringing it
back to handle up to 6 TiB wide levels
…ring'

This renaming does not show up in the history, since I rebased the pull request with
(sadly only half of) the change.
This is much easier to guess on first look as to what it is supposed to do.
This function was only used in the test suite for Nested Sweeping while it
is a source-code copy of '.as_ptr(...)'. In fact, I spend quite a bit of
time tracking down a small bug in this that did not exist in '.as_ptr(...)'.
@SSoelvsten SSoelvsten added ✨ code quality Uncle Bob would be proud 📁 internal This is where the ✨magic✨happens labels Dec 3, 2023
@SSoelvsten SSoelvsten self-assigned this Dec 3, 2023
It is kind of odd that the other bit-wise operators on pointers affects the
flag, but this is not the case for bit-wise negation. Hence, by making it into a
boolean negation we can differentiate between the two 'semantics'.
These functions are not used anywhere
…'level_info'

This is to match the change with 'ptr_uint64'.
For almost all its uses, the 'operator!' is safe-guarded. Hence, we may as well
just move it inside to clean up the rest of the code-base.
In this specific case, we know that both values are terminals. Hence, we
may use the slightly faster 'operator~' function. It also flips the flag.
But, the surrounding 'op(...)' removes it again
@SSoelvsten SSoelvsten force-pushed the internal/ptr_uint64/more_id_bits branch from 7d7d198 to 39ce35c Compare December 3, 2023 22:38
Copy link

codecov bot commented Dec 3, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (b82ab6a) 96.974% compared to head (39ce35c) 96.972%.

Files Patch % Lines
src/adiar/internal/data_types/level_info.h 0.000% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##              main      #591       +/-   ##
=============================================
- Coverage   96.974%   96.972%   -0.003%     
=============================================
  Files           84        84               
  Lines         5883      5878        -5     
=============================================
- Hits          5705      5700        -5     
  Misses         178       178               

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Dec 3, 2023

🟢 Regression Test (QBF 'breakthrough/3x4_19_bwnib')

'origin/internal/ptr_uint64/more_id_bits' is a change in performance of 0.76% (stdev: 2.27%).

... origin/main origin/internal/ptr_uint64/more_id_bits
Mean 47864.00 47502.00
Standard Deviation 1084.32 146.67

Number of samples: 4

Copy link

github-actions bot commented Dec 3, 2023

🟢 Regression Test (QBF 'breakthrough/3x5_11_bwnib')

'origin/internal/ptr_uint64/more_id_bits' is a change in performance of 0.75% (stdev: 1.18%).

... origin/main origin/internal/ptr_uint64/more_id_bits
Mean 12946.20 12849.60
Standard Deviation 152.45 27.38

Number of samples: 5

Copy link

github-actions bot commented Dec 3, 2023

🟢 Regression Test (QBF 'ep/8x8_7_e-8-1_p-3-4_bwnib')

'origin/internal/ptr_uint64/more_id_bits' is a change in performance of 1.32% (stdev: 3.68%).

... origin/main origin/internal/ptr_uint64/more_id_bits
Mean 16917.80 16694.20
Standard Deviation 622.24 57.03

Number of samples: 5

Copy link

github-actions bot commented Dec 3, 2023

🟡 Regression Test (Picotrav 'arbiter')

'origin/internal/ptr_uint64/more_id_bits' is a change in performance of -0.61% (stdev: 0.97%).

... origin/main origin/internal/ptr_uint64/more_id_bits
Mean 62571.00 62953.75
Standard Deviation 604.33 153.90

Number of samples: 4

Copy link

github-actions bot commented Dec 3, 2023

🟢 Regression Test (QBF 'domineering/5x5_13_bwnib')

'origin/internal/ptr_uint64/more_id_bits' is a change in performance of 2.07% (stdev: 3.61%).

... origin/main origin/internal/ptr_uint64/more_id_bits
Mean 43356.20 42460.00
Standard Deviation 1565.62 115.73

Number of samples: 5

Copy link

github-actions bot commented Dec 3, 2023

🔴 Regression Test (Picotrav 'mem_ctrl')

'origin/internal/ptr_uint64/more_id_bits' is a change in performance of -1.03% (stdev: 0.39%).

... origin/main origin/internal/ptr_uint64/more_id_bits
Mean 110973.75 112111.25
Standard Deviation 428.08 272.82

Number of samples: 4

@SSoelvsten SSoelvsten merged commit d7f91ad into main Dec 3, 2023
47 of 54 checks passed
@SSoelvsten SSoelvsten deleted the internal/ptr_uint64/more_id_bits branch December 3, 2023 23:33
@SSoelvsten SSoelvsten restored the internal/ptr_uint64/more_id_bits branch December 3, 2023 23:39
Copy link

github-actions bot commented Dec 3, 2023

🟡 Regression Test (Picotrav 'adder')

'origin/internal/ptr_uint64/more_id_bits' is a change in performance of -6.35% (stdev: 11.74%).

... origin/main origin/internal/ptr_uint64/more_id_bits
Mean 9228.60 9814.30
Standard Deviation 57.59 1152.43

Number of samples: 10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ code quality Uncle Bob would be proud 📁 internal This is where the ✨magic✨happens
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant