feat: optimize functions for getting text dimension #199
Conversation
Codecov Report
@@ Coverage Diff @@
## master #199 +/- ##
==========================================
+ Coverage 99.91% 99.91% +<.01%
==========================================
Files 94 99 +5
Lines 1165 1198 +33
Branches 281 290 +9
==========================================
+ Hits 1164 1197 +33
Partials 1 1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #199 +/- ##
==========================================
+ Coverage 99.91% 99.91% +<.01%
==========================================
Files 94 102 +8
Lines 1165 1221 +56
Branches 281 293 +12
==========================================
+ Hits 1164 1220 +56
Partials 1 1
Continue to review full report at Codecov.
|
|
Deploy preview for superset-ui ready! Built with commit edcd795 |
kristw
changed the title
| const dimensions = texts.map(text => { | ||
| // Empty string | ||
| if (text.length === 0) { | ||
| return { height: 0, width: 0 }; |
There was a problem hiding this comment.
instead of this special case, why not initialize the cache with:
{ "": { height: 0, width: 0 } }
then we don't need to check both cases every time (and i'd imagine the O(1) cache lookup would be about the same speed as the string length compare)
There was a problem hiding this comment.
I generally agree with this, but the W3C SVGGraphicsElement spec has a method named getBBox (that we use on line 9), so this instance may be an acceptable exception
| @@ -0,0 +1,41 @@ | |||
| export default class LazyFactory<T extends HTMLElement | SVGElement> { | |||
| private cache = new Map< | |||
There was a problem hiding this comment.
Is cache the right name for this? Seems like it's storing each node for its entire lifecycle, and not really being used as a cache
There was a problem hiding this comment.
I don't have strong opinion here. What would you suggest it be called?
There was a problem hiding this comment.
Maybe containers? I don't feel too strongly either.
There was a problem hiding this comment.
It is hashed by containers but not storing the containers, so probably should not be called containers.
I can rename to activeNodes.
* feat(datatable): render html correctly n * fix(sanitize string): sanitize string before parsing n
馃弳 Enhancements
branching off from ideas in #173.
getTextDimensionby lazy creation and deletion of thesvgandtextnode to avoid creating multiple nodes and reusing existing one when applicable.getMultipleTextDimensionsThis function takes an array of text and compute dimensions in batch, reusing the text node and apply caching.LazyFactorythat keeps counter of how many time a certain svg/text node is requested, hashed by container. Removal will decrease this counter and really remove the item when counter reaches 0.getTextDimensionandgetMultipleTextDimensionswill leave the invisible nodes around for 500ms before removal to support reusability.