Skip to content

perf(escapeHTML): use multiple replace - #178

Closed
SukkaW wants to merge 1 commit into
hexojs:masterfrom
SukkaW:perf-escape-unescape
Closed

perf(escapeHTML): use multiple replace#178
SukkaW wants to merge 1 commit into
hexojs:masterfrom
SukkaW:perf-escape-unescape

Conversation

@SukkaW

@SukkaW SukkaW commented Jan 28, 2020

Copy link
Copy Markdown
Member

Benchmark: https://runkit.com/sukkaw/5e3003ffe7e84c0013f6210d

const Benchmark = require('benchmark');
const Suite = new Benchmark.Suite;
const html = `<p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p>`;
const htmlEntityMap = {
  '&': '&amp;',
  '<': '&lt;',
  '>': '&gt;',
  '"': '&quot;',
  '\'': '&#39;',
  '`': '&#96;',
  '/': '&#x2F;',
  '=': '&#x3D;'
};

Suite.add('Map replacement', () => {
  html.replace(/[&<>"'`/=]/g, a => htmlEntityMap[a]);
}).add('Multiple replace', () => {
  html.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/\\/g, '&#39;').replace(/`/g, '&#96;').replace(/\//g, '&#x2F').replace(/=/g, '&#x3D');
}).on('cycle', function(event) {
  console.info(String(event.target));
}).run();

On my local computer, Multiple replace is the slower one under Node.js 10 but it is the faster one under Node.js 12 & 13:

image

@SukkaW
SukkaW requested a review from a team January 28, 2020 10:24
@SukkaW
SukkaW force-pushed the perf-escape-unescape branch from a721cb7 to 087af71 Compare January 28, 2020 10:25
@coveralls

coveralls commented Jan 28, 2020

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 96.979% when pulling 087af71 on SukkaW:perf-escape-unescape into 4e7c24e on hexojs:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.01%) to 96.979% when pulling 087af71 on SukkaW:perf-escape-unescape into 4e7c24e on hexojs:master.

@seaoak

seaoak commented Jan 30, 2020

Copy link
Copy Markdown
Member

I modified the source text to include all special characters.

const html = `<p class="foo">Hello "world&".</p><p class="foo">Hello "world\"".</p><p class="foo">Hello "world\`".</p><p class="foo">Hello "world=".</p><p class="foo">Hello "world\\".</p><p class="foo">Hello "world&".</p><p class="foo">Hello "world\"".</p><p class="foo">Hello "world\`".</p><p class="foo">Hello "world=".</p><p class="foo">Hello "world\\".</p><p class="foo">Hello "world&".</p><p class="foo">Hello "world\"".</p><p class="foo">Hello "world\`".</p><p class="foo">Hello "world=".</p><p class="foo">Hello "world\\".</p><p class="foo">Hello "world&".</p><p class="foo">Hello "world\"".</p><p class="foo">Hello "world\`".</p><p class="foo">Hello "world=".</p><p class="foo">Hello "world\\".</p><p class="foo">Hello "world&".</p><p class="foo">Hello "world\"".</p><p class="foo">Hello "world\`".</p><p class="foo">Hello "world=".</p><p class="foo">Hello "world\\".</p><p class="foo">Hello "world".</p><p class="foo">Hello "world".</p>`;

The result of benchmark with Node v12 is reversal.
Multiple replace is slower.

SS20200130b0 perf_escapeHTML_v12

The result of benchmark with Node v13 is same as SukkaW's report.
Multiple replace is faster.

SS20200130a0 perf_escapeHTML_v13

It is certain that my sample source text is unusual HTML.
I only show that this patch is not all-around.
Because this patch sacrifices readability,
the profit seems too little.

@SukkaW

SukkaW commented Jan 16, 2022

Copy link
Copy Markdown
Member Author

More performance benchmark needs to be done before we merge this.

@SukkaW
SukkaW marked this pull request as draft January 16, 2022 04:23
@SukkaW

SukkaW commented May 31, 2025

Copy link
Copy Markdown
Member Author

Superseded by #421

@SukkaW SukkaW closed this May 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants