Fix 1037 (rebased against master) #1377
Conversation
|
Thanks for your pull request and interest in making D better, @timotheecour! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. |
| @@ -142,7 +156,8 @@ class DependencyResolver(CONFIGS, CONFIG) { | |||
|
|
|||
| // get the current config/version of the current dependency | |||
| sizediff_t childidx = package_indices[basepack]; | |||
| if (all_configs[childidx].length == 1 && all_configs[childidx][0] == CONFIG.invalid) { | |||
| auto child = configs[childidx]; | |||
| if (child.allConfigs == [CONFIG.invalid]) { | |||
There was a problem hiding this comment.
Produces a GC allocation in the inner loop.
There was a problem hiding this comment.
did you measure whether it actually had a performance impact?
also, if it does, StaticArray(Config.invalid) could be used
There was a problem hiding this comment.
oh wow that actually uses the GC... who would have thought if you could simply put that array in a separate line as static immutable to not do that.
There was a problem hiding this comment.
Yes, in the sense that I got huge speedups with all allocations removed from that loop. Pretty stupid that this is not optimized away by the compiler automatically.
There was a problem hiding this comment.
dlang/druntime#2093 [WIP] core.array.staticArray static array litteral: staticArray(1, 2, 3) #2093
There was a problem hiding this comment.
Definitely an improvement, although I still think that the compiler should be smarter about array literals. It could for example simply infer the possible constness of the literal, and if it is not required to be mutable, it can safely allocate a static array instead of issuing a GC allocation (marking it as @nogc, too).
just a rebase of #1154 from @WebFreak001 against master