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

Add Compile command. #248

Closed
wants to merge 8 commits into from
Closed

Conversation

calavera
Copy link
Contributor

@calavera calavera commented Mar 7, 2019

This command allows people to compile a wasm module to machine code ahead
of time. It dumps the data in the cache, so future runs can use it.

It also changes the Cache trait to use Strings as keys, so it's more flexible than using the WasmHash object directly.

I've added a --from-cache flag to the Run command to allow loading modules directly from the cache without having access to the wasm or wat files. I'm not super sold on this flag.

I'm super open to suggestions and other options.

Enjoy this doggo taking a bath:

@Hywan
Copy link
Contributor

Hywan commented Mar 8, 2019

Thanks for the great PR!

I think the ideal workflows should be the following:

  1. wasmer run <file.wasm> to run a Wasm binary file. A cache of the compiled version is created on-the-fly, and re-used for the next runs,
  2. wasmer compile <file.wasm> to compile a Wasm file in the cache ahead of time, then simply run wasm run <file.wasm> would fetch it from the cache, as if it was a “next run” for the previous item.

Based on these assumptions, the --from-cache option is not necessary.

Thoughts?

@calavera
Copy link
Contributor Author

calavera commented Mar 8, 2019

@Hywan that means that you need the wasm file where you compile and where you execute the module, that's not always the same servers.

@xmclark xmclark self-requested a review March 8, 2019 16:52
Copy link
Contributor

@xmclark xmclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@calavera this seems like your use case:

  1. Compile wasm to cache file with Wasmer, cache is ouput somewhere accessible
  2. persist cache for later...do something else...kill some time...send it over the network etc.
  3. Load the cache back into Wasmer (maybe on a different machine) and run it

This seems like a valid use case. This is an ahead-of-time compilation workflow for hot runs. I think the addition of the Compile subcommand and the --from-cache option is fine. The confusing part about --from-cache is that we require a wasm file for the Run command. Ignoring the required option seems confusing. Maybe a dedicated subcommand for running from a cache?

@calavera
Copy link
Contributor Author

calavera commented Mar 8, 2019

Ignoring the required option seems confusing. Maybe a dedicated subcommand for running from a cache?

I thought about that too, and it would make sense because this feature is dependent on the cache. Do you have any suggestions for the subcomand name @xmclark ?

What about this?

wasmer cache exec WASM_HASH

@lachlansneff
Copy link
Contributor

lachlansneff commented Mar 8, 2019

My opinion is that we provide two cache subcommands:

wasmer cache generate <wasm file> -o/--output this/is/a/dir

  • takes an optional output directory argument

and

wasmer cache run foo/bar/hash

  • Takes a path to the cache as the argument.

@calavera
Copy link
Contributor Author

calavera commented Mar 8, 2019

I'm also realizing that this might need the wasm path, or at least a name for the module to be executed. I'm thinking about what's the best way to solve that.

@calavera
Copy link
Contributor Author

calavera commented Mar 8, 2019

My opinion is that we provide two cache subcommands

I like this suggestion

@calavera
Copy link
Contributor Author

calavera commented Mar 8, 2019

@lachlansneff, @xmclark I've implemented those two subcommands, tell me if this makes sense:

- `wasmer cache generate` to generate a machine object.
- `wasmer cache run` to run a cached object.

The run command requires two arguments. The first one is the name of the
program to run, to pass it as args[0] to the module, and the key for the
object in the cache. It also allows to send extra arguments to pass them
to the module. Example:

$ wasmer cache run nginx VERY_LONG_HASH -- -c /opt/nginx/config

I can write documentation for those subcommands if this is a good way to implement this.

Copy link
Contributor

@xmclark xmclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

.map_err(|e| format!("Can't execute module from cache: {:?}", e))?;

let name = String::from(options.module_name.to_str().unwrap());
run_wasm_module(&module, name, &options.args)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small nitpick: the module_name is unecessary on the CLI. Can you pass the hash as a string to run_wasm_module in place of the name? This will allow us to remove the module_name field from CacheRun too.

This way all we have to type is:
$ wasmer cache run VERY_LONG_HASH -- -c /opt/nginx/config

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do that, I was not sure if it'd be ok 👍

@lachlansneff
Copy link
Contributor

bors try

bors bot added a commit that referenced this pull request Mar 8, 2019
@bors
Copy link
Contributor

bors bot commented Mar 8, 2019

try

Build succeeded

@calavera
Copy link
Contributor Author

I've removed that extra argument in the wasmer cache run command. Do you have any conventions to write documentation? I don't see any docs in this repo besides the readme.

@calavera
Copy link
Contributor Author

@xmclark @lachlansneff it looks like Circle failed to download a dependency, and I cannot trigger a new job.

@lachlansneff
Copy link
Contributor

@calavera Seems like a Cargo.lock issue? Try resetting it?

This command allows people to compile a wasm module to machine code
ahead of time. It dumps the data in the cache, so future runs can use it.

Signed-off-by: David Calavera <david.calavera@gmail.com>
This is more flexible than using the WasmHash as the key.

Signed-off-by: David Calavera <david.calavera@gmail.com>
After a module is compiled, it can be loaded from the cache without
providing a wasm object.

Signed-off-by: David Calavera <david.calavera@gmail.com>
Signed-off-by: David Calavera <david.calavera@gmail.com>
Signed-off-by: David Calavera <david.calavera@gmail.com>
- `wasmer cache generate` to generate a machine object.
- `wasmer cache run` to run a cached object.

The run command requires two arguments. The first one is the name of the
program to run, to pass it as args[0] to the module, and the key for the
object in the cache. It also allows to send extra arguments to pass them
to the module. Example:

```
$ wasmer cache run nginx VERY_LONG_HASH -- -c /opt/nginx/config
```

Signed-off-by: David Calavera <david.calavera@gmail.com>
@calavera
Copy link
Contributor Author

it looks like rebasing master did the trick @lachlansneff 🎊

@xmclark
Copy link
Contributor

xmclark commented Mar 11, 2019

bors try

bors bot added a commit that referenced this pull request Mar 11, 2019
@bors
Copy link
Contributor

bors bot commented Mar 11, 2019

try

Build failed

Use the module key in the cache as the name.

Signed-off-by: David Calavera <david.calavera@gmail.com>
@calavera
Copy link
Contributor Author

bors try

@bors
Copy link
Contributor

bors bot commented Mar 12, 2019

🔒 Permission denied

Existing reviewers: click here to make calavera a reviewer

@bjfish
Copy link
Contributor

bjfish commented Mar 12, 2019

bors try

bors bot added a commit that referenced this pull request Mar 12, 2019
@bors
Copy link
Contributor

bors bot commented Mar 12, 2019

try

Build succeeded

@syrusakbary
Copy link
Member

A small update on this PR: we are gathering more feedback (and learnings) about the compilation/caching runtime needs, to make sure we expose the right API for it.

We will comment here as soon as we are 100% clear on it (hopefully soon!)

@MarkMcCaskey MarkMcCaskey added this to Needs review in Wasmer Reviews Nov 25, 2019
@syrusakbary
Copy link
Member

We are on the final steps of rewriting Wasmer from scratch and this issue is now solved in the refactor (we have a compile subcommand).

@syrusakbary
Copy link
Member

Hi @calavera!

We finally added support for precompiling wasm files to native object files (or JIT).
The usage is quite easy:

Compile:

wasmer compile python.wasm -o python.so --native

Run:

wasmer run python.so

Note: if you use the native engine, the startup time to load them is just in the order of nanoseconds even for big wasm files.

Wasmer Reviews automation moved this from Needs review to Done Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature! 📦 lib-deprecated About the deprecated crates
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants