[GitHub] [pinot] richardstartin opened a new pull request #7412: Reduce allocation in JSONPath evaluation

[GitBox]

richardstartin opened a new pull request #7412:
URL: https://github.com/apache/pinot/pull/7412


   ## Description
   
   The JSONPath library allocates a stateless object and reinitialises 
configuration once per JSON path invocation, which contributes to high 
allocation rate per JSON path evaluation and shows up in profiles. This is 
another piece of profiling motivated low hanging fruit.
   
   <img width="753" alt="Screenshot 2021-09-08 at 14 04 56" 
src="https://user-images.githubusercontent.com/16439049/132522686-6f7276a1-e6db-4cd8-bef6-88401b6bd554.png";>
   <img width="1451" alt="Screenshot 2021-09-08 at 14 05 24" 
src="https://user-images.githubusercontent.com/16439049/132522702-cbe4978b-4cec-4c36-a5f8-80dab2f41ff3.png";>
   
   This change allocates `ParseContextImpl` once during 
`JsonFunctions.<clinit>` and also avoids varargs allocations, which reduces 
memory allocated per JSON path evaluation by 152 bytes and improves throughput 
by ~5%.
   
   Before:
   ```
   Benchmark                                                     (_json)        
                  (_jsonPath)   Mode  Cnt     Score     Error   Units
   JSONPathEvaluationBenchmark.get                               DEFAULT  
$.tool.jsonpath.creator.location[2]  thrpt    5   736.002 ±   5.899  ops/ms
   JSONPathEvaluationBenchmark.get:·gc.alloc.rate.norm           DEFAULT  
$.tool.jsonpath.creator.location[2]  thrpt    5  3056.006 ±   0.001    B/op
   ```
   
   After
   ```
   Benchmark                                                     (_json)        
                  (_jsonPath)   Mode  Cnt     Score     Error   Units
   JSONPathEvaluationBenchmark.get                               DEFAULT  
$.tool.jsonpath.creator.location[2]  thrpt    5   770.663 ±   4.739  ops/ms
   JSONPathEvaluationBenchmark.get:·gc.alloc.rate.norm           DEFAULT  
$.tool.jsonpath.creator.location[2]  thrpt    5  2904.006 ±   0.001    B/op
   ```
   
   ```java
   @State(Scope.Benchmark)
   public class JSONPathEvaluationBenchmark {
   
     public enum JSON {
       DEFAULT("{\n"
           + "    \"tool\": \n"
           + "    {\n"
           + "        \"jsonpath\": \n"
           + "        {\n"
           + "            \"creator\": \n"
           + "            {\n"
           + "                \"name\": \"Jayway Inc.\",\n"
           + "                \"location\": \n"
           + "                [\n"
           + "                    \"Malmo\",\n"
           + "                    \"San Francisco\",\n"
           + "                    \"Helsingborg\"\n"
           + "                ]\n"
           + "            }\n"
           + "        }\n"
           + "    }\n"
           + "}");
   
       private final String json;
   
       JSON(String json) {
         this.json = json;
       }
   
       @Override
       public String toString() {
         return json;
       }
     }
   
     @Param
     private JSON _json;
   
     @Param("$.tool.jsonpath.creator.location[2]")
     private String _jsonPath;
   
     @Setup(Level.Trial)
     public void setup() {
       // avoid bad quality cache from dominating
       CacheProvider.setCache(new Cache() {
         private final Map<String, JsonPath> _map = new HashMap<>();
         @Override
         public JsonPath get(String key) {
           return _map.get(key);
         }
   
         @Override
         public void put(String key, JsonPath value) {
           _map.put(key, value);
         }
       });
     }
   
     @Benchmark
     public Object get() {
       return JsonFunctions.jsonPath(_json.toString(), _jsonPath);
     }
   }
   ```
   
   ## Upgrade Notes
   Does this PR prevent a zero down-time upgrade? (Assume upgrade order: 
Controller, Broker, Server, Minion)
   * [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete 
the section below on Release Notes)
   
   Does this PR fix a zero-downtime upgrade introduced earlier?
   * [ ] Yes (Please label this as **<code>backward-incompat</code>**, and 
complete the section below on Release Notes)
   
   Does this PR otherwise need attention when creating release notes? Things to 
consider:
   - New configuration options
   - Deprecation of configurations
   - Signature changes to public methods/interfaces
   - New plugins added or old plugins removed
   * [ ] Yes (Please label this PR as **<code>release-notes</code>** and 
complete the section on Release Notes)
   ## Release Notes
   <!-- If you have tagged this as either backward-incompat or release-notes,
   you MUST add text here that you would like to see appear in release notes of 
the
   next release. -->
   
   <!-- If you have a series of commits adding or enabling a feature, then
   add this section only in final commit that marks the feature completed.
   Refer to earlier release notes to see examples of text.
   -->
   ## Documentation
   <!-- If you have introduced a new feature or configuration, please add it to 
the documentation as well.
   See 
https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
   -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org