Content Extensions: split the state machines to minimize prefix states

[Benjamin Poulain]

Content Extensions: split the state machines to minimize prefix states

[Benjamin Poulain]

Created attachment 249800 [details]
Patch

[Benjamin Poulain]

Comment on attachment 249800 [details]
Patch

Oops, I forgot a unrelated file in my xcodeproject.

[Benjamin Poulain]

Created attachment 249842 [details]
Patch

[Alex Christensen]

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]

Committed r182211: <http://trac.webkit.org/changeset/182211>

[Benjamin Poulain]

(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.