Recompiled and runned the unittests
Diff Detail
Diff Detail
- Repository
- R237 KConfig
- Branch
- simplifyParam
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 20093 Build 20111: arc lint + arc unit
Comment Actions
I'd make the types static const, or at least const.
Also are you sure that here std::map is better than QHash?
Comment Actions
static will add a Mutex in the generated assembly (as static variables in c++ are now thread safe) and I'm not sure this is a good thing for const - reading variables. I'll try another approach so we only initialize it once.
Comment Actions
I’m not. I’m concerned that the introduction of thread safety by adding a
static member will make the compiler slower.
Comment Actions
Thread safe doesn't necessarily mean there will be a mutex... Depends if it's a literal type or not for a start, also it's generally implemented using the double locking pattern with atomics so if there's a mutex it's likely to be involved only one time on first call of the function.
| src/kconfig_compiler/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. ;-) | |
| src/kconfig_compiler/kconfig_compiler.cpp | ||
|---|---|---|
| 1020 | 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. | |
| 1024 | 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 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). | |
Comment Actions
David,
Thanks for the detailed explanation, I'm currently reworking most of the
patches here. About the optimizations - I know the linear search is a bad
optimization, but it adds legibility, that's what I tried to do.
I tried to emulate the python if value in vector that is really easy to
read, compared to the current code.