xtermjs / xterm.js

A terminal for the web
https://xtermjs.org/
MIT License
17.54k stars 1.62k forks source link

Should we pre-compute values behind getters in CellData? #1991

Closed Tyriar closed 5 years ago

Tyriar commented 5 years ago

https://github.com/xtermjs/xterm.js/pull/1878 introduced CellData, I posed this question in that PR and it got deferred so we can merge:

Should code, width, combined and chars be pre-computed here? Looks like most are used whenever they're pulled in. If we did that we could get rid of the get functions.

jerch commented 5 years ago

@Tyriar Prolly a good idea. I was a bit uneasy about those and their lifecycle, thats the reason why often used values from the buffer ended up with doubled getters (on the buffer and on CellData). Maybe CellData could drilldown the values once in ctor or something like that.

jerch commented 5 years ago

@Tyriar Replaced getWidth and getChars in a playground branch with precomputed values on CellData during Bufferline.loadCell. Not promising yet, it adds ~5% runtime to my ls -lR /usr call. Needs more investigation, especially once we have a buffer status addon, as this might see a benefit from precalc'ed values.

Tyriar commented 5 years ago

@jerch feel free to close if you don't think it's a good idea, was just capturing a thought I had with this.

jerch commented 5 years ago

@Tyriar Yeah lets close this, gonna keep an eye on it anyway.