Skip to content

Conversation

@walterddr
Copy link
Contributor

@walterddr walterddr commented Feb 8, 2022

Cache the Literal converted Geometry instead of computing it each time the Geometry function is called.

This

  • save memory for processing literal transform function (previously it was done via Array.fill)
  • save compute from Geometry function

In general this should apply to any chained call to LiteralTransformFunction with block --> it should be considered as a fake array instead fo a real, replicated one. This is out of scope of this PR but something to keep in mind.

@codecov-commenter
Copy link

codecov-commenter commented Feb 8, 2022

Codecov Report

Merging #8167 (81dde0e) into master (df39bda) will decrease coverage by 6.64%.
The diff coverage is 57.31%.

❗ Current head 81dde0e differs from pull request most recent head c11d097. Consider uploading reports for the commit c11d097 to get more accurate results

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #8167      +/-   ##
============================================
- Coverage     71.01%   64.36%   -6.65%     
  Complexity     4314     4314              
============================================
  Files          1624     1580      -44     
  Lines         84873    82964    -1909     
  Branches      12791    12584     -207     
============================================
- Hits          60273    53402    -6871     
- Misses        20453    25744    +5291     
+ Partials       4147     3818     -329     
Flag Coverage Δ
integration1 ?
integration2 ?
unittests1 67.42% <57.31%> (-0.04%) ⬇️
unittests2 14.11% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...sform/function/BaseBinaryGeoTransformFunction.java 54.83% <54.83%> (ø)
...spatial/transform/function/StDistanceFunction.java 85.71% <57.14%> (+1.23%) ⬆️
...spatial/transform/function/StContainsFunction.java 57.14% <60.00%> (-18.72%) ⬇️
...eospatial/transform/function/StWithinFunction.java 57.14% <60.00%> (-18.72%) ⬇️
...eospatial/transform/function/StEqualsFunction.java 80.00% <100.00%> (-2.15%) ⬇️
...a/org/apache/pinot/common/metrics/MinionMeter.java 0.00% <0.00%> (-100.00%) ⬇️
...g/apache/pinot/common/metrics/ControllerMeter.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pinot/common/metrics/BrokerQueryPhase.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pinot/common/metrics/MinionQueryPhase.java 0.00% <0.00%> (-100.00%) ⬇️
...ache/pinot/server/access/AccessControlFactory.java 0.00% <0.00%> (-100.00%) ⬇️
... and 374 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df39bda...c11d097. Read the comment docs.

@walterddr walterddr changed the title [draft] example to optimize geometry serializer usage for literal Optimize geometry serializer usage when literal is available Feb 9, 2022
@walterddr walterddr force-pushed the optimize_st_literal branch 2 times, most recently from a50d4a9 to b38053d Compare February 10, 2022 05:36
@walterddr walterddr marked this pull request as ready for review February 10, 2022 16:22
@Jackie-Jiang Jackie-Jiang merged commit 3d37a13 into apache:master Feb 15, 2022
xiangfu0 pushed a commit to xiangfu0/pinot that referenced this pull request Feb 23, 2022
…8167)

Cache the Literal converted Geometry instead of computing it each time the Geometry function is called. 

This 
- save memory for processing literal transform function (previously it was done via Array.fill)
- save compute from Geometry function

In general this should apply to any chained call to LiteralTransformFunction with block --> it should be considered as a fake array instead fo a real, replicated one. This is out of scope of this PR but something to keep in mind.
@walterddr walterddr deleted the optimize_st_literal branch December 6, 2023 16:21
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.

3 participants