Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try using Preferences.jl instead of Pkg.build #524

Closed
wants to merge 10 commits into from
Closed

Conversation

simonbyrne
Copy link
Member

This is an attempt to switch to using Preferences.jl instead of relying on environment variables. It is somewhat orthogonal to #513, but there are some design decisions that will necessarily interact.

This is based on discussion #483.

Instead of using environment variables, users would now call

julia -e 'using MPI; MPI.use_system_binary()'

There are a couple of pain points that I think would need to be addressed:

  • We can't query or set preferences at Pkg.build time. This means that things we used to do (e.g. query environment variables, or run the code generator for unknown ABIs) will no longer work
  • As such, we can only support known ABIs at this time.
  • Setting the preferences requires installing and precompiling MPI, running the above code, then precompiling again. This is not an ideal workflow.

cc: @carstenbauer @giordano @staticfloat @vchuravy

@simonbyrne simonbyrne closed this Dec 6, 2021
@simonbyrne simonbyrne reopened this Dec 6, 2021
@staticfloat
Copy link
Contributor

We can't query or set preferences at Pkg.build time. This means that things we used to do (e.g. query environment variables, or run the code generator for unknown ABIs) will no longer work

Why can't you do this at precompile time instead? What's special about Pkg.build() time?

@simonbyrne
Copy link
Member Author

Possibly? What happens if we modify the preferences? Is there a way to trigger a second round of precompilation?

@staticfloat
Copy link
Contributor

staticfloat commented Dec 7, 2021 via email

@simonbyrne
Copy link
Member Author

Ah. So to clarify, the hash is based on the state of the preferences after precompilation, not before? i.e. so if I call @set_preferences! during precompilation, this won't trigger another round of precompilation when I load the package again?

@simonbyrne simonbyrne closed this Dec 7, 2021
@simonbyrne simonbyrne reopened this Dec 7, 2021
@staticfloat
Copy link
Contributor

So to clarify, the hash is based on the state of the preferences after precompilation, not before?

That is correct. Every time you call @load_preference(key), the macro will check to see if you are currently compiling. If you are, it will mark that preference name as a compile-time preference. Then, after the whole compilation is finished, it will write out your .ji file and embed a hash of the preferences into the header of the .ji file. Then, when searching .ji files, we reject any previously-compiled .ji files if their preferences hash doesn't match what was used when it was last compiled.

All that being said, you're absolutely responsible for ensuring that the state of your package is consistent, and it can get really confusing if a top-level definition changes halfway through precompilation. Example:

module PrefsExample
using Preferences

const provider = something(
    @load_preference("provider", nothing),
    "default",
)::String

function do_work()
    @static if provider == "default"
        @info("do default thing")
    else
        @info("do something else")
    end
end

# Set a DIFFERENT preference value
@set_preferences!("provider" => "special")

end # module PrefsExample

If you compile this once, do_work() will print do default thing, and it won't recompile itself automatically. But if you then do something else that causes recompilation (even something as simple as a .ji file falling out of the LRU cache due to having many other environments that use this package) suddenly the behavior will change. So my point is to make things as simple and straightforward as possible; if you can, only read the preference once, and only write it once, at well-defined points in your control flow. I know some people like to allow for environment variables to influence these things, perhaps by doing something like:

const provider = something(
    @load_preference("provider", nothing),
    get(ENV, "PROVIDER", nothing),
    "default",
)::String

But I really dislike this for the following reasons:

  • If you set this manually once during build time, if it needs to be recompiled at some time in the future and you neglect to set it, the result will be wrong.
  • It's global, affecting all environments. Preferences should be per-environment.
  • It's not easily reproducible across machines.

It's much better to provide the user a Package.set_provider() method that tells the user "Preference set; please restart Julia" or something, if it's a compile-time preference.

@simonbyrne
Copy link
Member Author

simonbyrne commented Dec 8, 2021

Thanks @staticfloat. One of the challenges I'm trying to solve is figuring out the ABI of the user-provided MPI library: this should be stored in the Preferences.jl, since we want to invalidate the precompile cache if it changes. So I want to do something like:

const abi = begin
    _abi = @load_preference("abi", nothing)
    if isnothing(_abi)
       _abi = find_abi()
       @set_preference!("abi" => _abi)
    end
    _abi
end

Is that reasonable usage?

Also, if the ABI is not known, then we need to generate a .jl file with the constants by compiling and running a C program. Previously we stored this information in deps; should we now use Scratch.jl for this? If so, does this mean that we have to put the

include(joinpath(@get_scratch!("constants"), "consts.jl"))

statement inside the __init__() function?

@staticfloat
Copy link
Contributor

Is that reasonable usage?

Yes, as long as you don't expect the ABI to change out from underneath the user. Your module only calls find_abi() at compile-time, so unless there's some other piece that changes the preference when the ABI changes, it will never recompile. You could do something like do a sanity check abi == find_abi() within your __init__(), set the preference again, then tell the user to restart Julia, I suppose.

should we now use Scratch.jl for this?

Yes, that's a great idea. Using Scratch doesn't require any real changes to when you do the work; you can do that in __init__() or at the top-level, either is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants