Review Request 127813: Process paths just once

Matthew Dawson matthew at mjdsystems.ca
Thu May 12 02:13:22 UTC 2016


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127813/#review95399
-----------------------------------------------------------



General +1 from me, just two things:

1) Can you split these two change into two commits?  One being the part about not removing file: twice, the second about removing duplicate homes?  If not, np.
2) Do you have any benchmark numbers about this?  Just looking at comparisions, it will take three comparisions each time to check the list, then at least one more comparision assuming they are all the same.  Versus only three comparisions on the old code.  And if any path does clean the string, the other comparisions are skipped in the old version.  That being said, if benchmarks suggest your version is faster in wall time (not callgrind counts), I have no problem taking it.

- Matthew Dawson


On May 4, 2016, 9:02 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127813/
> -----------------------------------------------------------
> 
> (Updated May 4, 2016, 9:02 p.m.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> -------
> 
> Just process every home path once, as the 3 alternatives will probably just be the same.
> Don't drop the file: prefix twice in every run.
> 
> 
> Diffs
> -----
> 
>   src/core/kconfiggroup.cpp 39d2441 
> 
> Diff: https://git.reviewboard.kde.org/r/127813/diff/
> 
> 
> Testing
> -------
> 
> Tests pass, apps work.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160512/d855b7f8/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list