[rekonq] Review Request 111521: Adding domain-specific hiding support to the adblocker

[Paul Rohrbach]

> On July 15, 2013, 7:41 p.m., Andrea Diamantini wrote:
> > It truly seems good! Many thanks! Just 2 questions: 
> > 1) how could you time so well things happening (generic hiding in 300-500 ms, domain-specific hiding 30-70µs)?
> > 2) how much this improve app memory footprint? (I mean: we were using just a string for each rule, now we have a string and 2 multihash...)
> 
> Paul Rohrbach wrote:
>     For the performace measurements I used the clock_gettime function from ctime (I got the idea from http://www.guyrutenberg.com/2007/09/22/profiling-code-using-clock_gettime/ ).
>     
>     All I can say about the memory footprint is, that it is increased. With my configurtation (EasyList, EasyList Germany, EasyPrivacy), one of the multihashs has about 30 entries, the other about 16000. You could probably use on of the valgrind tools for exact figures, but when I tested it, valgrind was to slow to be able to display the startpage in one hour...
>     But it would also be increased if the rules are stored as a list of strings, because at the moment, the domain-specific rules are just thrown away. I did this actually before I used the hash and it was very slow, because every rule had to be checked for every request.
>     
>

I managed to start rekonq with the massif tool (it worked, when I started it with about:blank as page). The memory difference seems to be about 3MB. Before applying my patch, rekonq used a peak value of 17.3MB for displaying about:blank, afterwards it used 20.0MB. The only way I could think of improving this, without changing the implementation, is allocating the rule strings on the heap and storing them as pointers in the QMultiHash. But I am not sure whether this would improve the memory footpring significantly, since QString has a copy-on-write mechanism which should prevent the duplication of the rule strings (since I don't now the exact mechanism, I can't say that for sure).

The patch also enables the blocking of https request, which I enabled before I began working on the element hiding and probably shouldn't be part of this patch. I can replace the patch, if you want me to.


- Paul


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111521/#review36004
-----------------------------------------------------------


On July 15, 2013, 2:10 p.m., Paul Rohrbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111521/
> -----------------------------------------------------------
> 
> (Updated July 15, 2013, 2:10 p.m.)
> 
> 
> Review request for rekonq.
> 
> 
> Description
> -------
> 
> Adding domain-specific hiding support to the adblocker.
> 
> The domain-specific rules are stored in a QMultiHash, where the key is the domain and the values are the rules for the specified domain. This causes redundancy, because on rule can be applied to more than one domain, but has a really fast look-up time. The code for the generic hiding has not changed and is just relocated.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt c749a69 
>   src/adblock/adblockelementhiding.h PRE-CREATION 
>   src/adblock/adblockelementhiding.cpp PRE-CREATION 
>   src/adblock/adblockmanager.h 3d329ad 
>   src/adblock/adblockmanager.cpp 51000a3 
> 
> Diff: http://git.reviewboard.kde.org/r/111521/diff/
> 
> 
> Testing
> -------
> 
> - it hides things
> - it is reasonably fast (generic hiding takes with my configuration 300-500ms, the domain-specific hiding only takes 30-70µs)
> 
> 
> Thanks,
> 
> Paul Rohrbach
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/rekonq/attachments/20130720/9aac7a6f/attachment.html>