Use base64 radix to support 5e11 to -5e11 #171
Merged
Conversation
This file contains 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
|
Previous benchmarks from #170 are roughly in the same ballpark. I don't see anything pointing to memory utilization going higher as well, but will run specific benchmarks for them once again. |
…rdersThemCorrectly
baldawar
changed the title
timbray
approved these changes
Jul 31, 2024
| @@ -325,7 +325,7 @@ private void deleteRangePattern(Range range) { | |||
|
|
|||
| // when bottom byte on forkOffset position < top byte in same position, there must be matches existing | |||
There was a problem hiding this comment.
You demonstrate courage by wading into this code
There was a problem hiding this comment.
its been the best source of learning a whole lot more of neat tricks within ruler.
| */ | ||
| class ComparableNumber { | ||
| // Use scientific notation to define the double number directly to avoid losing Precision by calculation |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue #, if available: #42
Description of changes:
This change move from hex to base64 for numbers and subsequently allows to now support 5e11 to -5e11 numberic range. We still use hex for CIDR, and so selectively change to the older hex based digit system when necessary.
In case it matters, 5e11 was picked intentionally as
5e11 * 1e6is the most we'd be able to support throughLongprimitive. Beyond we trigger integer overflows. As solution, we either have to find a different normalization method, or switch to BigDecimal. I expect BigDecimal math to have a performance hit. I chose to not tackle these in the interest of time.Apologies for any silly errors in the diff. This change was a bit rushed.
Benchmark / Performance (for source code changes):
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.