Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix performance of splitCssText #1615

Merged
merged 25 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a065eaf
Fix bug where the right split point was not being picked for the 3rd …
eoghanmurray Dec 18, 2024
e2fe660
Add test to put splitCssText through it's paces with a large file
eoghanmurray Dec 17, 2024
2f663d4
Introduce a limit which causes the 'efficiently' test to fail
eoghanmurray Dec 18, 2024
8ba3b54
Fix that it wasn't able to find a split when both halves were identical
eoghanmurray Dec 17, 2024
61e8b5f
Fix poor 'crawling' performance in this part of the algorithm for lar…
eoghanmurray Dec 17, 2024
3474cb2
Need to take larger jumps to be efficient; use the scaling factor to …
eoghanmurray Dec 17, 2024
906dd3b
Add changeset
eoghanmurray Dec 18, 2024
e0fb33d
Presuming the match on a character is more eficient than the indexOf …
eoghanmurray Dec 18, 2024
f5fcfa5
Fix eslint
eoghanmurray Dec 18, 2024
a797e96
Update puppeteer to try to solve the following issue in github actions:
eoghanmurray Dec 19, 2024
defd363
Bump Chrome to see if it solves following issue in github actions:
eoghanmurray Dec 19, 2024
ad42cba
Both Firefox and the 131 version of Chrome I've upgraded to remove de…
eoghanmurray Dec 20, 2024
dec4ad5
Drop chrome down again as getting overwhelmed with issues so will see…
eoghanmurray Dec 20, 2024
7d1e100
Bump all puppeteer to match the LTS version of chrome we've just spec…
eoghanmurray Dec 20, 2024
0c809e1
Fix json error
eoghanmurray Dec 20, 2024
2bae508
The previous version combo didn't install, trying next one up
eoghanmurray Dec 20, 2024
c43d695
Still getting 'zygote_host_impl_linux.cc(128)] No usable sandbox!' so…
eoghanmurray Dec 22, 2024
e16e2ac
Keep searching through https://github.com/puppeteer/puppeteer/blob/5d…
eoghanmurray Dec 22, 2024
c0bf8f8
Following https://chromium.googlesource.com/chromium/src/+/main/docs/…
eoghanmurray Dec 22, 2024
417a831
Continuing with https://chromium.googlesource.com/chromium/src/+/main…
eoghanmurray Dec 22, 2024
a72a28e
Another bump to find minimum version to remove the 'No Usable Sandbox…
eoghanmurray Dec 30, 2024
a677fb6
Final bump to top of https://github.com/puppeteer/puppeteer/blob/5d72…
eoghanmurray Dec 30, 2024
9086b94
Further bump of Chrome as issue still isn't solved - this previously …
eoghanmurray Dec 30, 2024
724a27b
Revert all puppeteer and chrome related changes as the problem was fi…
eoghanmurray Jan 2, 2025
0fbf355
I keep accidentally forgetting camelCase
eoghanmurray Jan 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/efficiently-splitCssText-1603.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"rrweb-snapshot": patch
"rrweb": patch
---

Improve performance of splitCssText for <style> elements with large css content - see #1603
65 changes: 54 additions & 11 deletions packages/rrweb-snapshot/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
* Browsers sometimes incorrectly escape `@import` on `.cssText` statements.
* This function tries to correct the escaping.
* more info: https://bugs.chromium.org/p/chromium/issues/detail?id=1472259
* @param cssImportRule

Check warning on line 83 in packages/rrweb-snapshot/src/utils.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/utils.ts#L83

[tsdoc/syntax] tsdoc-param-tag-missing-hyphen: The @param block should be followed by a parameter name and then a hyphen
* @returns `cssText` with browser inconsistencies fixed, or null if not applicable.
*/
export function escapeImportStatement(rule: CSSImportRule): string {
Expand Down Expand Up @@ -456,36 +456,79 @@

/**
* Maps the output of stringifyStylesheet to individual text nodes of a <style> element
* performance is not considered as this is anticipated to be very much an edge case
* (javascript is needed to add extra text nodes to a <style>)
* which occurs when javascript is used to append to the style element
* and may also occur when browsers opt to break up large text nodes
* performance needs to be considered, see e.g. #1603
*/
export function splitCssText(
cssText: string,
style: HTMLStyleElement,
): string[] {
const childNodes = Array.from(style.childNodes);
const splits: string[] = [];
let iterLimit = 0;
if (childNodes.length > 1 && cssText && typeof cssText === 'string') {
const cssTextNorm = normalizeCssString(cssText);
let cssTextNorm = normalizeCssString(cssText);
const normFactor = cssTextNorm.length / cssText.length;
for (let i = 1; i < childNodes.length; i++) {
if (
childNodes[i].textContent &&
typeof childNodes[i].textContent === 'string'
) {
const textContentNorm = normalizeCssString(childNodes[i].textContent!);

Check warning on line 478 in packages/rrweb-snapshot/src/utils.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/rrweb-snapshot/src/utils.ts#L478

[@typescript-eslint/no-non-null-assertion] Forbidden non-null assertion.
for (let j = 3; j < textContentNorm.length; j++) {
// find a substring that appears only once
let j = 3;
for (; j < textContentNorm.length; j++) {
if (
// keep consuming css identifiers (to get a decent chunk more quickly)
textContentNorm[j].match(/[a-zA-Z0-9]/) ||
// substring needs to be unique to this section
textContentNorm.indexOf(textContentNorm.substring(0, j), 1) !== -1
) {
continue;
}
break;
}
for (; j < textContentNorm.length; j++) {
const bit = textContentNorm.substring(0, j);
if (cssTextNorm.split(bit).length === 2) {
const splitNorm = cssTextNorm.indexOf(bit);
// this substring should appears only once in overall text too
const bits = cssTextNorm.split(bit);
let splitNorm = -1;
if (bits.length === 2) {
splitNorm = cssTextNorm.indexOf(bit);
} else if (
bits.length > 2 &&
bits[0] === '' &&
childNodes[i - 1].textContent !== ''
) {
// this childNode has same starting content as previous
splitNorm = cssTextNorm.indexOf(bit, 1);
}
if (splitNorm !== -1) {
// find the split point in the original text
for (let k = splitNorm; k < cssText.length; k++) {
if (
normalizeCssString(cssText.substring(0, k)).length === splitNorm
) {
let k = Math.floor(splitNorm / normFactor);
for (; k > 0 && k < cssText.length; ) {
iterLimit += 1;
if (iterLimit > 50 * childNodes.length) {
// quit for performance purposes
splits.push(cssText);
return splits;
}
const normPart = normalizeCssString(cssText.substring(0, k));
if (normPart.length === splitNorm) {
splits.push(cssText.substring(0, k));
cssText = cssText.substring(k);
cssTextNorm = cssTextNorm.substring(splitNorm);
break;
} else if (normPart.length < splitNorm) {
k += Math.max(
1,
Math.floor((splitNorm - normPart.length) / normFactor),
);
} else {
k -= Math.max(
1,
Math.floor((normPart.length - splitNorm) * normFactor),
);
}
}
break;
Expand Down
58 changes: 58 additions & 0 deletions packages/rrweb-snapshot/test/css.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import postcss, { type AcceptedPlugin } from 'postcss';
import { JSDOM } from 'jsdom';
import { splitCssText, stringifyStylesheet } from './../src/utils';
import { applyCssSplits } from './../src/rebuild';
import * as fs from 'fs';
import * as path from 'path';
import type {
serializedElementNodeWithId,
BuildCache,
Expand Down Expand Up @@ -105,10 +107,16 @@ describe('css splitter', () => {
// as authored, e.g. no spaces
style.append('.a{background-color:black;}');

// test how normalization finds the right sections
style.append('.b {background-color:black;}');
style.append('.c{ background-color: black}');

// how it is currently stringified (spaces present)
const expected = [
'.a { background-color: red; }',
'.a { background-color: black; }',
'.b { background-color: black; }',
'.c { background-color: black; }',
];
const browserSheet = expected.join('');
expect(stringifyStylesheet(style.sheet!)).toEqual(browserSheet);
Expand Down Expand Up @@ -137,6 +145,28 @@ describe('css splitter', () => {
}
});

it('finds css textElement splits correctly with two identical text nodes', () => {
const window = new Window({ url: 'https://localhost:8080' });
const document = window.document;
// as authored, with comment, missing semicolons
const textContent = '.a { color:red; } .b { color:blue; }';
document.head.innerHTML = '<style></style>';
const style = document.querySelector('style');
if (style) {
style.append(textContent);
style.append(textContent);

const expected = [textContent, textContent];
const browserSheet = expected.join('');
expect(splitCssText(browserSheet, style)).toEqual(expected);

style.append(textContent);
const expected3 = [textContent, textContent, textContent];
const browserSheet3 = expected3.join('');
expect(splitCssText(browserSheet3, style)).toEqual(expected3);
}
});

it('finds css textElement splits correctly when vendor prefixed rules have been removed', () => {
const style = JSDOM.fragment(`<style></style>`).querySelector('style');
if (style) {
Expand Down Expand Up @@ -169,6 +199,34 @@ describe('css splitter', () => {
expect(splitCssText(browserSheet, style)).toEqual(expected);
}
});

it('efficiently finds split points in large files', () => {
const cssText = fs.readFileSync(
path.resolve(__dirname, './css/benchmark.css'),
'utf8',
);

const parts = cssText.split('}');
const sections = [];
for (let i = 0; i < parts.length - 1; i++) {
if (i % 100 === 0) {
sections.push(parts[i] + '}');
} else {
sections[sections.length - 1] += parts[i] + '}';
}
}
sections[sections.length - 1] += parts[parts.length - 1];

expect(cssText.length).toEqual(sections.join('').length);

const style = JSDOM.fragment(`<style></style>`).querySelector('style');
if (style) {
sections.forEach((section) => {
style.appendChild(JSDOM.fragment(section));
});
}
expect(splitCssText(cssText, style)).toEqual(sections);
});
});

describe('applyCssSplits css rejoiner', function () {
Expand Down
4 changes: 2 additions & 2 deletions packages/rrweb/test/__snapshots__/replayer.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ file-cid-3

.css-added-at-200 { position: fixed; top: 0px; right: 0px; left: 4rem; z-index: 15; flex-shrink: 0; height: 0.25rem; overflow: hidden; background-color: rgb(17, 171, 209); }

.css-added-at-200.alt { height: 0.25rem; background-color: rgb(190, 232, 242); opacity: 0; transition: opacity 0.5s ease 0s; }
.css-added-at-200.alt { height: 0.25rem; background-color: rgb(190, 232, 242); opacity: 0; transition: opacity 0.5s ease-in 0.1s; }

.css-added-at-1000-deleted-at-2500 { display: flex; flex-direction: column; min-width: 60rem; min-height: 100vh; color: blue; }

Expand Down Expand Up @@ -152,7 +152,7 @@ file-cid-3

.css-added-at-200 { position: fixed; top: 0px; right: 0px; left: 4rem; z-index: 15; flex-shrink: 0; height: 0.25rem; overflow: hidden; background-color: rgb(17, 171, 209); }

.css-added-at-200.alt { height: 0.25rem; background-color: rgb(190, 232, 242); opacity: 0; transition: opacity 0.5s ease 0s; }
.css-added-at-200.alt { height: 0.25rem; background-color: rgb(190, 232, 242); opacity: 0; transition: opacity 0.5s ease-in 0.1s; }

.css-added-at-200.alt2 { padding-left: 4rem; }
"
Expand Down
2 changes: 1 addition & 1 deletion packages/rrweb/test/events/style-sheet-rule-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const events: eventWithTime[] = [
tagName: 'style',
attributes: {
_cssText:
'.css-added-at-200 { position: fixed; top: 0px; right: 0px; left: 4rem; z-index: 15; flex-shrink: 0; height: 0.25rem; overflow: hidden; background-color: rgb(17, 171, 209); }.css-added-at-200.alt { height: 0.25rem; background-color: rgb(190, 232, 242); opacity: 0; transition: opacity 0.5s ease 0s; }.css-added-at-200.alt2 { padding-left: 4rem; }',
'.css-added-at-200 { position: fixed; top: 0px; right: 0px; left: 4rem; z-index: 15; flex-shrink: 0; height: 0.25rem; overflow: hidden; background-color: rgb(17, 171, 209); }.css-added-at-200.alt { height: 0.25rem; background-color: rgb(190, 232, 242); opacity: 0; transition: opacity 0.5s ease-in 0.1s; }.css-added-at-200.alt2 { padding-left: 4rem; }',
'data-emotion': 'css',
},
childNodes: [
Expand Down
Loading