Open curran opened 9 months ago
Yeah it can get very unperformant for large documents. It's a large refactor that I haven't had time to do yet. Out of curiosity, how large was your document where you saw this behavior?
Oh sorry! I totally should have included the sample document. It was actually not very large. This is the one I was using for testing:
import { select, transition, easeLinear } from 'd3';
import data from './data.csv';
import './styles.css';
export const main = (container) => {
const svg = select(container)
.selectAll('svg')
.data([1])
.join('svg')
.attr('width', container.clientWidth)
.attr('height', container.clientHeight);
const slow = transition().duration(2000);
const fast = transition().duration(200).ease(easeLinear);
svg
.selectAll('circle')
.data(data, (d) => d.id)
.join(
(enter) =>
enter
.append('circle')
.attr('r', 0)
.attr('cx', (d) => d.x)
.attr('cy', (d) => d.y)
.attr('fill', (d) => d.fill)
.call((selection) => {
selection
.transition(slow)
.delay((d, i) => i * 200)
.attr('r', (d) => d.r);
}),
(update) =>
update.call((selection) => {
selection
.transition(fast)
.attr('cx', (d) => d.x)
.attr('cy', (d) => d.y)
.attr('r', (d) => d.r)
.attr('fill', (d) => d.fill);
}),
(exit) =>
exit.call((selection) =>
selection.transition(slow).attr('r', 0).remove(),
),
)
.attr('opacity', 880 / 1000);
};
For my use case, the font size and font family never change, and I suspect measuring of the text may be the bottleneck here. I'd love to be able to pass a flag like fixedFont = true
to tell the implementation not to worry about dynamic font sizes.
Here are some ideas from ChatGPT on how to optimize:
To optimize the given code with the fixedFont
flag, we need to make several adjustments. The primary objective is to ensure that font-related computations and updates are done only once, given that the font size and family are fixed. This approach will significantly reduce the overhead in scenarios where these attributes are constant.
Here's the optimized code with comments explaining the changes:
// Add the fixedFont flag in the constructor to indicate whether the font is fixed
export class TextState extends LineBasedState<Array<TagSpan>> {
private fixedFont: boolean;
public constructor(view: EditorView, fixedFont = false) {
super(view);
this.fixedFont = fixedFont;
// Other constructor code remains the same...
}
// Override the shouldUpdate method to account for the fixedFont flag
private shouldUpdate(update: ViewUpdate) {
// If the font is fixed, we do not need to update for theme changes
if (this.fixedFont && this.themeChanged()) {
return false;
}
// Rest of the method remains the same...
}
// Modify updateImpl to conditionally handle font information based on fixedFont
private updateImpl(state: EditorState, changes?: ChangeSet) {
// Other updateImpl code remains the same...
// Clear and recalculate the font info map only if the font is not fixed
if (!this.fixedFont && this.themeChanged()) {
this._fontInfoMap.clear();
}
// Rest of the updateImpl method...
}
public measure(context: CanvasRenderingContext2D): {
charWidth: number;
lineHeight: number;
} {
// If the font is fixed and font info is already calculated, use the cached value
if (this.fixedFont && this._fontInfoMap.size > 0) {
const fontInfo = this._fontInfoMap.values().next().value;
return {
charWidth: context.measureText("_").width,
lineHeight: fontInfo.lineHeight,
};
}
// Rest of the measure method remains the same...
}
public beforeDraw() {
// Clear the font info map only if the font is not fixed
if (!this.fixedFont) {
this._fontInfoMap.clear();
}
// Rest of the beforeDraw method...
}
private getFontInfo(tags: string): FontInfo {
// Use the cached font info if the font is fixed and already calculated
if (this.fixedFont && this._fontInfoMap.size > 0) {
return this._fontInfoMap.values().next().value;
}
// Rest of the getFontInfo method...
}
// Rest of the TextState class...
}
// Usage example:
// const textState = new TextState(view, true); // Set fixedFont to true
Constructor Modification: Added the fixedFont
flag as a class property to determine if font-related computations should be optimized.
Conditional Theme Change Handling: In the shouldUpdate
and updateImpl
methods, the theme change check is skipped if fixedFont
is true.
Measure Optimization: In the measure
method, if fixedFont
is true and font information is already calculated, it uses the cached value instead of recalculating.
Conditional Font Info Cache Clearance: In beforeDraw
and getFontInfo
, the font info map is not cleared or recalculated if fixedFont
is true and already populated.
This approach ensures that font-related computations are minimized when the fixedFont
flag is set, leading to improved performance under the specified conditions.
I adopted this extension, but then noticed that it causes substantial performance issues. I thought I'd drop a line here with what I noticed.
To reproduce: select some text, and hold down the arrow key so it selects more and more.
The updates take around 72ms.
Specifically, these two calls within the minimap extension:
measure
anddrawLine
Specifically,
drawLine
is not re-using DOM elements: