RFC: Pass custom snoopfile and arbitrary julia flags to compile_incremental

[Pbellive]

Hi,
Thanks for all the recent work on PackageCompiler! I've made a couple changes to compile_incremental that make it a bit more general and make it work smoothly for my use case. I think they could be useful to the general community so I've put them in a small PR. Of course feel free to ignore if you don't think these changes fit with the direction PackageCompiler is headed. This PR makes three changes:

1. Allow the user to pass their own "snoopfile" to compile_incremental. I think it makes sense to use a package's tests as the default thing to run to generate a precompile file but I don't understand why the option to use one's own snoopfile isn't there. I know I can just call snoop to generate a precompile file from my own snoopfile and then pass it to compile_incremental but then I lose the convenience of having one call to compile_incremental that calls snoop_packages and then the compilation itself all in one go.

2. Allow arbitrary keyword arguments to be passed to compile_incremental that just get passed straight through to run_julia. This allows the user pass any julia command line flags to run_julia. My main use case for this has been to compile with --cpu-target=x86-64 so I can generate a system image that works across a cluster with heterogeneous hardware. I could just add a cpu_target keyword argument to compile_incremental but it seems convenient to allow passing whatever flags one might like, such as changing the bounds checking setting or the math-mode for example.

3. This last change is a hack but a useful hack I think. I've found that if the Distributed standard lib is initialized in the precompile file then compilation crashes with a Task cannot be serialized error. In practice this has meant that I haven't been able to compile any packages that depend on Distributed. However, packages that use functionality from Distributed can still be compiled. I had been making this work manually. For example, to compile JSON.jl I would run compile_incremental(:JSON), which crashed when it ran julia with -output-o. I would then take a look at .../PackageCompiler/packages/incremental_precompile.jl, which in this example has the following lines:

using Pkg, Test, Distributed, FixedPointNumbers, OffsetArrays, JSON, DataStructures, Mmap, Unicode, Dates, PackageCompiler, Sockets
for Mod in [Pkg, Test, Distributed, FixedPointNumbers, OffsetArrays, JSON, DataStructures, Mmap, Unicode, Dates, PackageCompiler, Sockets]
    isdefined(Mod, :__init__) && Mod.__init__()
end

I would then manually edit the file to remove the references to Distributed from the section shown and then run compile_incremental(nothing,".../PackageCompiler/packages/incremental_precompile.jl"). This PR automates the removal of the references to Distributed by changing line 47 of snooping.jl to

packages = join(setdiff(used_packages,["Distributed"]), ", ")

This is of course a total hack but I imagine it could make a lot more packages compile-able right now.

[SimonDanisch]

So --cpu-target=x86-64 doesn't make sense, we should actually make sure, that nobody is passing that flag and error if someone does! This is because it's an incremental compile, working off the current system image. What we should do is, to allow passing different system images, and use PackageCompilers system image caching infrastructure to make it easy to work compile from different images!

[SimonDanisch]

For the other point, we should make it possible to pass a blacklist of packages!

[SimonDanisch]

Concerning 1), I'd like to come up with the convention to have a snoopfile.jl in the package!
Since the interface is Package oriented right now, it'd be weird to pass a package and an external snoopfile. That would also make it harder for others to compile the same package! So if it is tightly bound to a package, it should live in the package.
I think it's either no package + and a simple script as snoopfile, or a package, that uses either runtests.jl or snoopfile.jl!

[SimonDanisch]

From the available flags, we should only pass these through:

"check-bounds"
#"code-coverage"
# "compile"
#"compiled-modules"
# "cpu-target"
"depwarn"
"handle-signals"
# "history-file"
# "machine-file"
"math-mode"
"optimize"
# "output-bc"
# "output-ji"
# "output-jitbc"
# "output-o"
# "output-unoptbc"
# "project"
# "startup-file"
"sysimage"
"sysimage-native-code" # not sure actually
#"trace-compile"
#"track-allocation"
"warn-overwrite"
"inline"

All others will yield undefined behaviour, if I'm not mistaken!

[Pbellive]

Thanks for the feedback! I like the idea of putting a snoopfile in packages. Would the idea be to check for a snoopfile in the package, use it if it exists, and fall back to runtests.jl if it doesn't?

I also like the package blacklist idea! Is it enough just to basically do what I've done in this PR but replace the hardcoded removal of Distributed by the removal of a user supplied list of packages? If so I can do that and submit a new PR. Not sure if you had something more sophisticated in mind.

I'm sorry, I don't think I understand your comments on setting the cpu-target. My initial understanding was that your recommendation would be to run PackageCompiler.build_sysimage to generate a new sys.so file using the desired cpu_target. Then use compile_incremental from there. I may be missing something but as far as I can tell, running build_sysimage with appropriate keyword arguments and a user image file generated by running snoop_packages just builds a backup image and then does the same thing as compile_incremental? Am I wrong about that? Is the plan for compile_incremental to do something different in the future?

Using either build_sysimage or compile_incremental I've found it necessary to set the cpu_target in the following scenario: I've downloaded and installed the generic linux binaries from julialang.org on two machines, one with a Haswell cpu and the other with an ivybridge. I've used compile_incremental on the Haswell machine to generate a new sysimage, then used that custom sysimage on both machines. This works fine if I build the sysimage by running julia with a cpu-target that's compatible with both machines. I've tested with cpu_target=x86-64 and ivybridge, the lowest common denominator between the machines. If I don't explicitly set the cpu target then I can't use the custom sysimage on the ivybridge machine. I get

ERROR: Unable to find compatible target in system image

when launching julia, as expected. I've seen the same behaviour using a julia installation built from source (recent master branch of julia) with the setting JULIA_CPU_TARGET=x86_64 in Make.user. So it's worked for me. Having said all that, it could be that what I've done is bad for reasons I don't understand and I've just been lucky. I also understand that adding this option makes it easy for users to pass a bad value for the cpu_target.

[SimonDanisch]

Would the idea be to check for a snoopfile in the package, use it if it exists, and fall back to runtests.jl if it doesn't?

Yup! And maybe also check for a snoop require - but that could be an additional step ;)

Is it enough just to basically do what I've done in this PR but replace the hardcoded removal of Distributed by the removal of a user supplied list of packages?

Yes, that sounds sufficient!

ERROR: Unable to find compatible target in system image

Ah that's weird...My assumption was, that it propagates the system image cpu_target from the generic binary... But I just checked on windows with the generic binaries, and my cpu_target in Base.JLOptions() is actually native...
Makes sense I guess, since that instructs what LLVM should emit at runtime.
Also, I didn't realize that you can mix x86-64 and native in one system image (since it's incremental and your prebuild system image must have been compiled with x86-64 and your target is by default native, one should end up with a mix) ... But why not, it's all assembly in the end, so might as well be possible to mix those.

But I guess it's pretty fragile to mix x86-64 and native in one binary, so it would still cleaner to pass a system image with the right cpu target ... the question is, how one can infer the cpu target from a system image.
I'd say we should improve PackageCompiler to have a better cache for all kinds of system images, and then we can maybe just pass cpu_target, and PackageCompiler will select the correct image from the cache, or build a new one!

[Pbellive]

Would the idea be to check for a snoopfile in the package, use it if it exists, and fall back to runtests.jl if it doesn't?

Yup! And maybe also check for a snoop require - but that could be an additional step ;)

Is it enough just to basically do what I've done in this PR but replace the hardcoded removal of Distributed by the removal of a user supplied list of packages?

Yes, that sounds sufficient!

Great. I'll make a new PR with those changes. Should be able to close this one. I'll also get rid of passing through arbitrary keyword arguments and just include the ones that make sense that are mentioned above.

Re the CPU target flag: A full understanding of the implications of mixing instructions for different architectures in one system image is over my head. However, from reading the docs on specifying multiple system image targets I would have thought mixing things would be fine as long as valid instructions for the current cpu can be found at runtime. Having said that I agree that it's cleaner to just pass the current system image with the right cpu_target into compile_incremental.