WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
143253
Content Extensions: split the state machines to minimize prefix states
https://bugs.webkit.org/show_bug.cgi?id=143253
Summary
Content Extensions: split the state machines to minimize prefix states
Benjamin Poulain
Reported
2015-03-30 21:37:18 PDT
Content Extensions: split the state machines to minimize prefix states
Attachments
Patch
(78.14 KB, text/plain)
2015-03-30 22:02 PDT
,
Benjamin Poulain
no flags
Details
Patch
(75.76 KB, patch)
2015-03-31 12:31 PDT
,
Benjamin Poulain
achristensen
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Benjamin Poulain
Comment 1
2015-03-30 22:02:53 PDT
Created
attachment 249800
[details]
Patch
Benjamin Poulain
Comment 2
2015-03-30 23:10:34 PDT
Comment on
attachment 249800
[details]
Patch Oops, I forgot a unrelated file in my xcodeproject.
Benjamin Poulain
Comment 3
2015-03-31 12:31:33 PDT
Created
attachment 249842
[details]
Patch
Alex Christensen
Comment 4
2015-03-31 15:34:35 PDT
Comment on
attachment 249842
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=249842&action=review
lgtm
> Source/WebCore/contentextensions/CombinedURLFilters.cpp:62 > + Vector<PrefixTreeVertex*, 128> prefixTreeVerticesForPattern;
Why 128 here? Is that just an arbitrary large number? It's not related to the number of possible edges in one PrefixTreeVertex, right?
> Source/WebCore/contentextensions/CombinedURLFilters.cpp:93 > + for (unsigned i = pattern.size(); i--;) {
This is beautiful. I especially like the fact that it needs post decrement :)
> Source/WebCore/contentextensions/CombinedURLFilters.cpp:125 > + ActiveNFASubtree& activeSubtree = activeStack.last(); > + for (;activeSubtree.iterator != activeSubtree.vertex->edges.end(); ++activeSubtree.iterator) {
activeSubtree is not used outside of the for loop. Why not put its declaration inside the loop?
Benjamin Poulain
Comment 5
2015-03-31 16:00:06 PDT
Committed
r182211
: <
http://trac.webkit.org/changeset/182211
>
Benjamin Poulain
Comment 6
2015-03-31 16:01:35 PDT
(In reply to
comment #4
)
> Comment on
attachment 249842
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=249842&action=review
> > lgtm > > > Source/WebCore/contentextensions/CombinedURLFilters.cpp:62 > > + Vector<PrefixTreeVertex*, 128> prefixTreeVerticesForPattern; > > Why 128 here? Is that just an arbitrary large number? It's not related to > the number of possible edges in one PrefixTreeVertex, right?
I made up that number :) It is the length of the pattern. I figured 128 should be bigger than most patterns.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug