Optimize JRT.compare2 hot paths and publish benchmark reports#477
Optimize JRT.compare2 hot paths and publish benchmark reports#477bertysentry wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5677cf8eef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else if (isComparisonNumber(o1String)) { | ||
| o1Number = new BigDecimal(o1String).doubleValue(); |
There was a problem hiding this comment.
isComparisonNumber() accepts arbitrarily long exponents, but the subsequent new BigDecimal(o1String) call is now unguarded; inputs like "1e2147483649" pass the predicate and then throw NumberFormatException (exponent overflow). Before this change, the conversion was inside a try/catch and compare2 fell back to string comparison, so this is a regression that can crash comparisons on malformed or extreme numeric-looking fields instead of evaluating them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR optimizes io.jawk.jrt.JRT.compare2 for common operand shapes (especially numeric comparisons), introduces a comparison-number fast check to avoid exception-driven coercion on hot paths, and adds the infrastructure to benchmark and publish those results via the Maven site + GitHub Pages.
Changes:
- Fast-path numeric vs numeric comparisons in
compare2and addisComparisonNumberfor cheap numeric-string detection. - Add unit tests covering numeric, numeric-string, mixed, and fallback string-comparison semantics.
- Add a JMH benchmark suite and a GitHub Actions workflow + site page to publish and display benchmark reports.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/io/jawk/jrt/JRT.java | Optimizes compare2 hot paths; introduces isComparisonNumber helper. |
| src/test/java/io/jawk/JRTTest.java | Adds focused compare2 semantic tests. |
| src/test/java/io/jawk/jrt/JRTComparisonNumberTest.java | Adds unit tests for isComparisonNumber acceptance/rejection cases (incl. hex rejection). |
| src/jmh/java/io/jawk/jrt/JRTCompare2Benchmark.java | Adds JMH microbenchmarks for common compare2 operand shapes. |
| pom.xml | Adds JMH version + benchmark profile; wires JMH sources into formatting/licensing and build steps. |
| src/site/xhtml/benchmarks.xhtml | Adds a benchmarks documentation page intended to load/present published JSON results. |
| src/site/site.xml | Adds navigation entry to the new benchmarks page. |
| src/site/resources/css/site.css | Adds styling for the benchmarks page layout/table. |
| CONTRIBUTING.md | Documents how to build/run benchmarks and how publishing works. |
| .github/workflows/benchmarks.yml | Adds a workflow to run JMH on tags/dispatch, update the site JSON index, and publish to GitHub Pages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function renderTemplate(host) { | ||
| host.innerHTML = [ | ||
| '<div ng-controller="BenchmarkController as vm" ng-cloak="">', | ||
| ' <div class="alert alert-info" ng-if="vm.loading">Loading published benchmark index...</div>', | ||
| ' <div class="alert alert-warning" ng-if="vm.error">{{vm.error}}</div>', |
| if (o1String.equals(o2String)) { | ||
| return mode == 0; | ||
| } |
| const content = await getText(new URL(normalizedPath, siteBase)); | ||
| if (content === null) { | ||
| return; | ||
| } | ||
| const outputPath = path.join(siteRoot, normalizedPath); |
Summary
compare2operand shapes inJRTand avoid exception-driven numeric coercion on the hot path.compare2and publish release benchmark data through GitHub Pages.Testing
mvn license:update-file-header formatter:format verify sitebenchmarks.htmlwith sample benchmark JSON