move shuffling of animals to be done on demand instead of during initialization

[KristofferC]

This package is heavily used throughout the ecosystem so optimizing its load time is quite important. The compilation time of the shuffle! call in the __init__ function adds quite a bit of overhead, so I moved this to be done the first time the variable is actually used instead.

Without this PR:

julia> @time using MacroTools
  0.067275 seconds (142.56 k allocations: 9.640 MiB, 61.33% compilation time)

With this PR:

julia> @time using MacroTools
  0.024947 seconds (28.79 k allocations: 3.159 MiB, 22.04% compilation time)

It also removed quite a bit of "noise" when profiling other packages load time that happens to depend on this pacakge recursively.

[cstjean]

Do you know why we shuffle in __init__ at the moment, and not at the top-level? I get that it would change behaviour (subsequent Julia sessions would have the same symbol order) but I fail to see how that would be significant.

For that matter, we could also shuffle here. It'd make subsequent calls return different results, but... why not?

I'd also consider moving the dict to the top level. Gensyms are never GC'ed anyway, so keeping a permanent dict so that the same gensym is replaced by the same animal would sound good to me.

I don't use animals, so maybe I'm missing something?

[cstjean]

I git blamed and it was your PR that brought it inside of __init__. At this point, I would favour a simpler strategy than introducing a lock and uninitialized variable, if possible?

Alternatively, the animals.txt file can just be line shuffled.

That would sound good to me too.

[cstjean]

Makes it easier to include this package in a sysimage since the default rng is not usable when outputting to a file.

Would shuffle!(MersenneTwister(123), animals) at the top level be sufficient?

[KristofferC]

I git blamed and it was your PR that brought it inside of init.

Yes, I know :D. The reason was because PackageCompiler.jl at that time had problems with using packages that calls rand methods at precompilation time. It should be fixed now though so maybe it could be put back to top-level.

I'll move it back up.

[cstjean]

CI doesn't seem to run anymore? 😦 Does anyone know how to fix it? If not, I'll just merge...

[KristofferC]

https://github.com/FluxML/MacroTools.jl/pull/170 ;)

[cstjean]

Thanks!

[KristofferC]

New release?