[Nepomuk] Review Request: FileWatch: Avoid calling the addWatch function recursively

[Simeon Bird]

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


My understanding is that this trades memory usage for speed - how much is the 
slowdown on installing watches initially? Can we get most of the memory savings
without the slowdown by just adding the watches in much smaller batches? 
eg. WATCHES_AT_ONCE = 5 or 10?

One other problem: this will still add subdirectories of filtered folders. 

eg, if I have:

dir/subdir/subsubdir
dir2/

and dir is filtered, we don't want to try to add watches for subdir or subsubdir (because they may be over a network and just finding their existence is potentially slow. Also if there are symlinks involved, we may iterate forever.); instead we want to jump straight to dir2. 

This should be easily fixed though. Just call FilterWatch in _k_add_Watches and only add the new iterator if it returns true. Can also resurrect addWatchNoCheck to avoid calling FilterWatch twice unnecessarily.

- Simeon Bird


On Nov. 30, 2012, 7:34 a.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107529/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2012, 7:34 a.m.)
> 
> 
> Review request for Nepomuk, Sebastian Trueg and Simeon Bird.
> 
> 
> Description
> -------
> 
>     FileWatch: Avoid calling the addWatch function recursively
>     
>     A watch needs to be added for each directory. If we try to add the
>     watches recursively, then at each level a new string is allocated which
>     consumes memory. This memory is eventually freed, but that doesn't
>     decrease the filewatch service's memory footprint.
>     
>     Additionally, this extra memory can be quite large depending on how your
>     directory is structed. For me it makes a memory difference of about
>     50mb, but bug reports indicate that it can go as high as 2gb.
>     
>     _k_addWatches() now only adds one watch and then calls itself. It
>     additionally traverses the file system tree in a depth first manner in
>     order to avoid extra memory allocations. (Breadth first costs more
>     memory)
>     
>     BUG: 310556
> 
> 
> This addresses bug 310556.
>     http://bugs.kde.org/show_bug.cgi?id=310556
> 
> 
> Diffs
> -----
> 
>   services/filewatch/kinotify.cpp e540f76 
> 
> Diff: http://git.reviewboard.kde.org/r/107529/diff/
> 
> 
> Testing
> -------
> 
> Memory footprint reduced from 65 to 17mb.
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/nepomuk/attachments/20121130/6a1de759/attachment.html>