maplibre-native icon indicating copy to clipboard operation
maplibre-native copied to clipboard

Track expression dependencies

Open TimSylvester opened this issue 1 year ago • 3 comments

When building expression trees, we assign and capture what categories of input they depend on, like feature data and zoom level.

When evaluating whether to update a layer, zoom changes only trigger an update if zoom is used in an expression somewhere within the style.

Pretty minor performance benefit on the iOS benchmark app.

main:

Average frame encoding time: 2.50 ms
Average frame rendering time: 6.49 ms

Average frame encoding time: 2.53 ms
Average frame rendering time: 6.50 ms

Average frame encoding time: 2.54 ms
Average frame rendering time: 6.52 ms

Average frame encoding time: 2.51 ms
Average frame rendering time: 6.41 ms


this:

Average frame encoding time: 2.43 ms
Average frame rendering time: 6.42 ms

Average frame encoding time: 2.43 ms
Average frame rendering time: 6.49 ms

Average frame encoding time: 2.49 ms
Average frame rendering time: 6.51 ms

Average frame encoding time: 2.41 ms
Average frame rendering time: 6.50 ms

TimSylvester avatar Feb 14 '24 23:02 TimSylvester

Bloaty Results (iOS) 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.1% +12.0Ki  +0.1% +16.0Ki    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-2113-compared-to-main.txt

github-actions[bot] avatar Feb 16 '24 01:02 github-actions[bot]

Bloaty Results 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.7%  +964Ki  +1.0%  +313Ki    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-2113-compared-to-main.txt

Compared to d38709084a9865fe0bb8300aec70ebf8243b3d43 (legacy)

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +20% +23.1Mi  +406% +24.3Mi    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-2113-compared-to-legacy.txt

github-actions[bot] avatar Feb 19 '24 19:02 github-actions[bot]

Codecov Report

Attention: Patch coverage is 66.16766% with 113 lines in your changes are missing coverage. Please review.

Project coverage is 59.02%. Comparing base (9614b56) to head (c0d719a).

Files Patch % Lines
src/mbgl/style/expression/compound_expression.cpp 59.13% 7 Missing and 40 partials :warning:
src/mbgl/style/expression/expression.cpp 39.13% 5 Missing and 9 partials :warning:
src/mbgl/style/expression/format_expression.cpp 31.57% 3 Missing and 10 partials :warning:
src/mbgl/style/expression/collator_expression.cpp 18.18% 0 Missing and 9 partials :warning:
src/mbgl/map/map_impl.cpp 70.00% 0 Missing and 6 partials :warning:
src/mbgl/style/expression/coercion.cpp 68.75% 1 Missing and 4 partials :warning:
src/mbgl/style/property_expression.cpp 0.00% 0 Missing and 4 partials :warning:
include/mbgl/style/property_expression.hpp 62.50% 0 Missing and 3 partials :warning:
src/mbgl/renderer/render_orchestrator.cpp 50.00% 0 Missing and 2 partials :warning:
src/mbgl/style/expression/find_zoom_curve.cpp 66.66% 0 Missing and 2 partials :warning:
... and 8 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2113      +/-   ##
==========================================
+ Coverage   58.38%   59.02%   +0.64%     
==========================================
  Files         575      575              
  Lines       28344    28489     +145     
  Branches    11376    11386      +10     
==========================================
+ Hits        16549    16816     +267     
+ Misses       4165     4032     -133     
- Partials     7630     7641      +11     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 19 '24 19:02 codecov[bot]

Could you add tests?

Done. I also expanded some existing test cases, as modifications to some areas with low coverage brought the average way down.

TimSylvester avatar Mar 06 '24 23:03 TimSylvester