perf: add cache support to evaluation in reearth/core #573
Conversation
✅ Deploy Preview for reearth-web ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
pyshx
changed the title
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #573 +/- ##
==========================================
+ Coverage 26.97% 27.10% +0.12%
==========================================
Files 746 748 +2
Lines 87439 87626 +187
Branches 1883 1916 +33
==========================================
+ Hits 23590 23750 +160
- Misses 63758 63783 +25
- Partials 91 93 +2
|
| } | ||
| } | ||
|
|
||
| export function getReferences(expression: string, feature?: any): string[] { |
There was a problem hiding this comment.
We have two function to parse expression. First is this function. Second is replaceVariables.
But these are very heavy task, so it will make app slower...
I think it's better to share the result of parsing with Expression class, and if we remove replaceVariables then performance will be more improved.
There was a problem hiding this comment.
I think it'll be hard to separate out alot the repetitive logic and avoid iterating over the string again in replaceVariables now that i've removed JP related evaluations.
| return new ConditionalExpression(styleExpression, feature, layer.defines).evaluate(); | ||
| const cacheKey = JSON.stringify([ | ||
| styleExpression, | ||
| getCacheableProperties(styleExpression, feature?.properties), |
There was a problem hiding this comment.
We don't have to instantiate Expression no more, but we become to need to parse expression every time...
There was a problem hiding this comment.
Hopefully this will be less heavy with the cache for getReferences in place.
Overview
This PR is for improving styling performance of reearth/core.