Conversation
Refactoring to prepare for runtime compilation support: - JsonPath is now a sealed interface permitting JsonPathInterpreted and JsonPathCompiled - JsonPathInterpreted contains the existing AST-walking evaluation logic - JsonPathCompiled stub added for future bytecode-compiled implementation - JsonPathCompiler stub added with in-memory compilation infrastructure - All 100 existing tests pass unchanged Next steps: Add tests and implementation for runtime-compiled JsonPath Co-authored-by: simbo1905 <simbo1905@60hertz.com>
Added JDK compiler-based runtime compilation for JsonPath:
- JsonPathExecutor: Public functional interface for compiled executors
- JsonPathHelpers: Public utility class with helper methods for generated code
(getPath, toComparable, compareValues, normalizeIdx, evaluateRecursiveDescent)
- JsonPathCompiler: Generates and compiles Java source code at runtime using
javax.tools.ToolProvider for in-memory compilation
- JsonPathCompiled: Executes compiled JsonPath queries
Code generation covers all JsonPath features:
- Property access (dot and bracket notation)
- Array indexing (positive and negative)
- Array slicing (start:end:step, reverse)
- Wildcards (* on objects and arrays)
- Filters (exists, comparison, logical AND/OR/NOT)
- Unions (multiple indices or properties)
- Recursive descent (.. for all descendants)
- Script expressions (@.length-1 pattern)
Added comprehensive test suite (JsonPathCompilerTest) with 40 tests verifying
compiled paths produce identical results to interpreted paths.
All 140 tests pass.
To compile a JsonPath for optimal performance:
JsonPath compiled = JsonPath.compile(JsonPath.parse("$.store.book[*].author"));
List<JsonValue> results = compiled.query(json);
Co-authored-by: simbo1905 <simbo1905@60hertz.com>
Updated README.md: - Added 'Runtime Compilation (Performance Optimization)' section - Documents how to use JsonPath.compile() for hot paths - Explains when to use compiled vs interpreted paths - Notes compilation is idempotent Updated AGENTS.md: - Documented the sealed interface architecture - Added new entry points for compilation-related classes - Added guidance for maintaining code generation Co-authored-by: simbo1905 <simbo1905@60hertz.com>
|
Cursor Agent can help with this pull request. Just |
Summary of ChangesHello @simbo1905, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a major performance enhancement to the JsonPath library by enabling runtime compilation of JsonPath expressions. Instead of solely relying on an AST-walking interpreter, users can now compile their JsonPath queries into optimized Java bytecode. This allows for faster execution, particularly beneficial for 'hot path' scenarios where the same query is run against many JSON documents. The changes involve a significant refactoring of the internal architecture to support both interpreted and compiled execution modes, ensuring flexibility and performance gains. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a major new feature: runtime compilation of JsonPath expressions to bytecode for improved performance. The architecture is well-designed, using a sealed interface JsonPath with JsonPathInterpreted and JsonPathCompiled implementations. The code generation and in-memory compilation are handled cleanly. The changes are accompanied by extensive documentation and a new test suite to ensure correctness by comparing compiled and interpreted results.
I've found a critical bug in the code generation for recursive descent that could lead to incorrect results, along with some dead code and areas for improvement in error handling and documentation clarity. Overall, this is a great addition, and with a few fixes, it will be a solid feature.
json-java21-jsonpath/src/main/java/json/java21/jsonpath/JsonPathCompiler.java
Show resolved
Hide resolved
json-java21-jsonpath/src/main/java/json/java21/jsonpath/JsonPathHelpers.java
Outdated
Show resolved
Hide resolved
json-java21-jsonpath/src/main/java/json/java21/jsonpath/JsonPathHelpers.java
Outdated
Show resolved
Hide resolved
json-java21-jsonpath/src/main/java/json/java21/jsonpath/JsonPathCompiler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbdd23bfba
ℹ️ 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".
json-java21-jsonpath/src/main/java/json/java21/jsonpath/JsonPathCompiler.java
Outdated
Show resolved
Hide resolved
…eter Addresses review comment: For unsupported recursive descent targets like $..[0:2], $..[?(@.x)], or $..['a','b'], the compiled code was incorrectly adding the current node to results. The interpreter logs and returns no matches for these cases. Changed to generate a no-op comment instead of adding to results, which matches the interpreter's semantics. Co-authored-by: simbo1905 <simbo1905@60hertz.com>
Fixes: 1. Remove unused evaluateRecursiveDescentFull() method from JsonPathHelpers 2. Remove unused collectAtPath() method from JsonPathHelpers 3. Fix README example: use 'compiled'/'interpreted' instead of undefined 'path' 4. Clarify in README that only limited script expressions are supported 5. Throw UnsupportedOperationException for unsupported script expressions instead of silently ignoring them All 140 tests pass. Co-authored-by: simbo1905 <simbo1905@60hertz.com>
This pull request contains changes generated by a Cursor Cloud Agent