Closed atilaneves closed 2 months ago
✅ PR OK, no changes in deprecations or warnings
Total deprecations: 0
Total warnings: 0
Build statistics:
statistics (-before, +after)
-executable size=5347800 bin/dub
-rough build time=64s
+executable size=5364184 bin/dub
+rough build time=63s
Looks good as far as I can see. Two additional things caught my eye:
getAllDependencies
now performs a redundant allocation of the returned list - since getAllDependenciesRange
is package
, we could now just scrap it and put the implementation directly into getAllDependencies
Package
instance, as getAllDependencies
is called in some places that look like they might be called often during a runTurns out there is actually code that mutates the dependencies of a Package
through its recipe
property, so the pre-computation won't work just like that, unfortunately. But on the other hand, for a certain project, this code got called more than 10 times per package on average when performing a build, so if this is something that turns up in a profiler, that should still be worthwhile to pursue.
I considered speeding up getAllDependencies
but it wasn't where the time was being spent. I even cached it at one point and it made no difference.
Package.getPackageConfigs
was taking 3 seconds for a particular dub package, which was made worse by the fact it was called multiple times with different arguments. The changes in this PR bring it down to just under 200ms, which IMHO is still slow.Investigating led me to realise that
determineDependencyConfigs
was being called over and over again for the exact same packages, especially common dependencies likemir-algorithm
andunit-threaded
. This PR does two things:First, it makes the array of package dependencies smaller via
.sort.uniq
. This cut down the time by half to 1.5s.Secondly, after taking a while to understand the inner workings of the function, it returns early for packages that have already been analysed. This brought it further down to the 100ms range.