AG-14117 - Fix tooltip forced relayout + misc optimisations. #3744
Conversation
alantreadway
changed the title
alantreadway
force-pushed
the
ag-14117/fix-tooltip-size-forcing-relayout
branch
from
ef17fc5 to
45469e6
Compare
alantreadway
changed the title
alantreadway
changed the title
| @@ -890,7 +890,9 @@ export class DataModel< | |||
| const columnScopes = new Set(def.scopes); | |||
| const columnScope = first(def.scopes); | |||
| const columnSource = sources.get(columnScope) as unknown[]; | |||
| const column = columnSource.map((valueDatum, datumIndex) => { | |||
| const column = new Array<unknown>(columnSource.length); | |||
There was a problem hiding this comment.
Use .map here - if you use new Array(length), it basically forces the array into a Map<number, element> behind the scenes. That was one of the big wins for the high performance work
There was a problem hiding this comment.
Based upon https://jsbench.me/t6m85tuhgv/1, our best option seems to be to just use new Array() without .map() - the main issue is likely that the closure can't be easily optimised at runtime for this particular case.
|
|
||
| const font = TextUtils.toFontString(this); | ||
| // Try to avoid this assignment, which typically always incurs a font switch cost. | ||
| if (ctx.font !== font) { |
There was a problem hiding this comment.
If this is showing as a hot path, it might be worth storing the resolved font on the instance too, and reading that to avoid the overhead of crossing the language barrier since the .font getter is C++
There was a problem hiding this comment.
This is probably ok, as with a prior iteration of this change the hotspot was significantly reduced with just this guard added. We'd need to track the current font in the renderContext for this to be accurate, which seems more complexity than I'd like to add right now.
https://ag-grid.atlassian.net/browse/AG-14117