Stream: git-wasmtime

Topic: wasmtime / Issue #2730 Remove some dependencies

Wasmtime GitHub notifications bot (Mar 17 2021 at 23:33):

pchickey commented on Issue #2730:

I approve these changes in cranelift-module. I'm not a maintainer of cranelift-codegen so I will defer overall approval to someone else - @cfallin maybe?

Wasmtime GitHub notifications bot (Mar 18 2021 at 00:16):

cfallin commented on Issue #2730:

It's somewhat surprising to me that changes in error-printing code would give such a large speedup in compile times. It's always welcome, of course, when speedups come -- but @bjorn3, do you have any sense of why this happens? I ask because it could be a symptom of something weirder going on behind the scenes -- for example, are we throwing a bunch of errors and doing work to capture backtraces but then silencing them somehow?

All other things being equal, the auto-derived error handling is kind of nice, but I won't fight too hard for it if we win by doing something lighter-weight.

Wasmtime GitHub notifications bot (Mar 18 2021 at 01:45):

pchickey commented on Issue #2730:

I believe, because thiserror is a proc macro that depends on the usual stack of syn and friends, that compiling those dependencies is the tentpole in embeddings which otherwise don't use proc macros.

Wasmtime GitHub notifications bot (Mar 18 2021 at 01:46):

pchickey edited a comment on Issue #2730:

I believe, because thiserror is a proc macro that depends on the usual stack of syn and friends, that compiling those dependencies is the tentpole in embeddings which otherwise don't use proc macros. So, we don't see this in wasmtime or other big applications, but I imagine that cg_clif is very careful about deps.

Wasmtime GitHub notifications bot (Mar 18 2021 at 01:46):

pchickey edited a comment on Issue #2730:

I believe, because thiserror is a proc macro that depends on the usual stack of syn and friends, that compiling those dependencies is the tentpole in embeddings which otherwise don't use proc macros. So, we don't see this in wasmtime or other big applications where we use that family or crates in several contexts, but I imagine that cg_clif is very careful about deps.

Wasmtime GitHub notifications bot (Mar 18 2021 at 02:00):

cfallin commented on Issue #2730:

Ah -- to clarify, @bjorn3, you mean the compile time of cg_clif, not the time that cg_clif takes to compile code?

(I was assuming the latter, and thus was quite confused about the impact of error-path code on the happy path!)

Wasmtime GitHub notifications bot (Mar 18 2021 at 02:01):

pchickey edited a comment on Issue #2730:

I believe, because thiserror is a proc macro that depends on the usual stack of syn and friends, that compiling those dependencies is the tentpole in embeddings which otherwise don't use proc macros. So, we don't see this in wasmtime or other big applications where we use that family of crates in several contexts, but I imagine that cg_clif is very careful about deps.

Wasmtime GitHub notifications bot (Mar 18 2021 at 06:57):

bjorn3 commented on Issue #2730:

Ah -- to clarify, @bjorn3, you mean the compile time of cg_clif, not the time that cg_clif takes to compile code?

Correct, I mean the time it takes to compile cg_clif itself. Sorry for the confusion.

but I imagine that cg_clif is very careful about deps.

Indeed. I try to avoid dependencies where possible and even use dependencies from the rustc-dev rustup component instead of crates.io where possible.

Wasmtime GitHub notifications bot (Mar 18 2021 at 18:46):

pchickey commented on Issue #2730:

Would you mind leaving a comment above the Display & Error impls that these are written by hand to avoid pulling in the thiserror dep? That may help anyone who sees them and gets the clever idea that they can be replaced by a macro.

Wasmtime GitHub notifications bot (Mar 19 2021 at 15:42):

bjorn3 commented on Issue #2730:

Done

Wasmtime GitHub notifications bot (May 04 2021 at 12:38):

bjorn3 commented on Issue #2730:

Rebased

Last updated: Dec 23 2024 at 12:05 UTC