| Summary: | Content Extensions: split the state machines to minimize prefix states | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||
| Component: | New Bugs | Assignee: | Benjamin Poulain <benjamin> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | achristensen, sam | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Benjamin Poulain
2015-03-30 21:37:18 PDT
Created attachment 249800 [details]
Patch
Comment on attachment 249800 [details]
Patch
Oops, I forgot a unrelated file in my xcodeproject.
Created attachment 249842 [details]
Patch
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? Committed r182211: <http://trac.webkit.org/changeset/182211> (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. |