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

Bringing Ruler packages back in sync #114

Merged
merged 6 commits into from
Oct 10, 2023
Merged

Bringing Ruler packages back in sync #114

merged 6 commits into from
Oct 10, 2023

Conversation

baldawar
Copy link
Collaborator

@baldawar baldawar commented Oct 6, 2023

Issue #, if available: N/A

Description of changes:

Both packages had bifurcated when wildcard and ruler internal improvements were happening in parallel. Aligning code-bases before move everyone to the OSS version under the cover.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -91,7 +90,7 @@ Set<NameStateWithPattern> transitionOn(String valString) {
boolean fieldValueIsNumeric = false;
if (hasNumeric.get() > 0) {
try {
final double numerically = JavaDoubleParser.parseDouble(valString);
Copy link
Contributor

Choose a reason for hiding this comment

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

can we keep our version here, I am pretty sure this one is faster than the standar double parser

Copy link
Collaborator Author

@baldawar baldawar Oct 6, 2023

Choose a reason for hiding this comment

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

Hey Rudy,
that's a good catch. let me double check the benchmark numbers. I thought JavaDoubleParser was faster too. I'll will respond on Tuesday (away until then).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't see any regressions or improvements within the benchmarks because most of the time is spent in ComparableNumber but when testing in isolation against the numbers within the benchmark data, I see this FastDoubleParser to be slightly better (45 seconds vs 50 seconds). Added it back for now.

@baldawar baldawar merged commit e97a9ce into main Oct 10, 2023
3 checks passed
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.

None yet

3 participants