D26126: Simplify param method: Return Early, Use a Map and Assert.

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.

INLINE COMMENTS

> kconfig_compiler.cpp:1020
> +            QLatin1String("double")}
> +        ).contains(type)) {
> +        return type;

This linear search doesn't look like an optimization to me. It would be better to incorporate this into the map, so that a single search is performed, rather than two.

> ervin wrote in kconfig_compiler.cpp:1024
> std::map looks like a bad idea here, either use QHash (preferred in massively based Qt code) or std::unordered_map.
> 
> Also for both temporaries you pay the price of their allocation and deallocation at each call. Even a mutex used at each call would do better. ;-)

I'm not 100% sure about std::map vs QHash given the number of elements, this would need benchmarking.

But I agree 100% that compiler-generated thread safety around a static is NOTHING compared to amount of nodes allocated to fill in a map or hash.

@tcanabrava Please have a look at https://gist.github.com/jboner/2841832
Locking an available mutex is 4 times faster than even fetching data from main memory (i.e. data which isn't yet in a CPU cache). This is many orders of magnitude faster than creating a filling a map or a hash full of QStrings. On top of that, compilers don't even generate a full-blown mutex for threadsafe statics, they generate a three-state atomic (a bit like Q_GLOBAL_STATIC does) (3 because not created, being created, already created).

The most performant solution is to turn the input string into a QByteArray and then perform a binary search in a C array of const char* (no allocations, even the very first time).
The most readable (but still good, unlike the current code in git) solution is a static map.

REPOSITORY
  R237 KConfig

REVISION DETAIL
  https://phabricator.kde.org/D26126

To: tcanabrava, ervin, dfaure
Cc: dfaure, ervin, GB_2, apol, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191224/75714c03/attachment.html>