Bug 28667

Summary:

Make XSSAuditor go fast

Product:

WebKit

Reporter:

Adam Barth <abarth>

Component:

WebCore JavaScript

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dbates, fishd, sam

Priority:

Keywords:

XSSAuditor

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch v1	none
Patch v1	eric: review+, eric: commit-queue-

Adam Barth

Reported 2009-08-23 01:27:26 PDT

Enabling the XSSAuditor slows down the intl1 page cycler slightly slower. We should make the auditor go faster. Patch forthcoming.

Attachments
Patch v1 (11.63 KB, patch) 2009-08-23 01:31 PDT, Adam Barth	no flags	Details Formatted Diff Diff
Patch v1 (8.49 KB, patch) 2009-08-23 10:51 PDT, Adam Barth	eric: review+ eric: commit-queue-	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Adam Barth

Comment 1 2009-08-23 01:31:36 PDT

Created attachment 38449 [details] Patch v1

Adam Barth

Comment 2 2009-08-23 01:37:57 PDT

Here are the perf numbers from my laptop (two runs for each condition, 15 cycles per run, warm disk cache): XSSAuditor disabled: 187.91, 188.07 XSSAuditor enabled (old): 191.60, 192.17 XSSAuditor enabled (new): 189.14, 189.85 By my calculations, the old auditor slowed down intl1 by 2% whereas the new auditor only slows it down by 0.8%. It's possible there is more room for optimization, but this change seems worth doing.

Mark Rowe (bdash)

Comment 3 2009-08-23 01:48:22 PDT

Comment on attachment 38449 [details] Patch v1 Rather than dropping the "const" from all of these functions that are conceptually cons I think it make more sense to mark m_memoizer as mutable instead.

Adam Barth

Comment 4 2009-08-23 10:05:24 PDT

Comment on attachment 38449 [details] Patch v1 Actually, I screwed up the memoization too.

Adam Barth

Comment 5 2009-08-23 10:51:18 PDT

Created attachment 38454 [details] Patch v1

Eric Seidel (no email)

Comment 6 2009-08-27 23:28:29 PDT

Comment on attachment 38454 [details] Patch v1 LGTM. I've got 2 nits though: 1. You mentioned that this is not technically "memoization" but rather caching. If so, let's call it CachingURLCanonicalizer and m_cachedURL (and maybe m_inputURL?) 2. seems strange to change some decodeHTMLentities to decodeEntities and others to decodeHTMLEntities. Pick one. :) I suggest "decodeEntities". I don't need to see this again.

Adam Barth

Comment 7 2009-08-28 01:08:12 PDT

Committed r47858: <http://trac.webkit.org/changeset/47858>

Note You need to log in before you can comment on or make changes to this bug.