Review Request 122071: Fix parsing environment hash

[Milian Wolff]

> On Jan. 15, 2015, 8:41 p.m., Milian Wolff wrote:
> > duchain/clangparsingenvironment.cpp, line 112
> > <https://git.reviewboard.kde.org/r/122071/diff/1/?file=342055#file342055line112>
> >
> >     I'm not good with hash functions, so I'm reluctant to accept this. I certainly see the issue though. Maybe we could instead do the following, which wouldn't be more work /and/ save to use:
> >     
> >     convert the hash to a list of QByteArrays (or std::strings) of the form "-DFOO=Bar", i.e. just as we need it later on in ParseSession anyways. This list can then be sorted for the hash function and we could keep using qHash + KDevHash::operator<<.
> 
> Kevin Funk wrote:
>     What about keeping a `QMap<QString, QString>` inside clangparsingenvironment.cpp instead -- this has multiple benefits: `hash()` will still be fast, and we don't accidently add keys multiple times (currently `addDefines` uses `QHash::unite` which may turn our hash into a multi-hash. Instead I'd rewrite `addDefines` to do `QMap::insert` on each kv-pair in `defines`.
>     
>     => After this we could use the original `hash()` function with now deterministic behavior.

yeah that would work as well. esp. if we have addDefines this might be required. Or one constructs the sorted list outside, once, and then just passes the list to the environment and keep it constant from there on? I don't really care which option we use, frankly :)


- Milian


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


On Jan. 15, 2015, 10:26 a.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122071/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2015, 10:26 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> The hash should stay the same, no matter what order of includes/defines is.
> 
> With this patch applied subsequent project loading time reduced from 150 to 10 seconds for the kdev-clang project. 
> 
> 
> Diffs
> -----
> 
>   duchain/clangparsingenvironment.cpp f73822f 
>   duchain/clangparsingenvironmentfile.cpp 88cbbe0 
>   tests/test_duchain.h 22bacd5 
>   tests/test_duchain.cpp cd21635 
> 
> Diff: https://git.reviewboard.kde.org/r/122071/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150115/c944ea6e/attachment.html>