Add Optional JIT compilation #91
Conversation
crates/node/src/compiler.rs
Outdated
| let cache = cache.clone(); | ||
|
|
||
| move || { | ||
| let context = Box::leak(Box::new(revmc::llvm::Context::create())); |
There was a problem hiding this comment.
Can I leak this, does that make sense? I guess I shouldn't need to since context will live for the whole while loop below.
There was a problem hiding this comment.
You should be able to use with_llvm_context and nest the implementation inside of the closure
| dbg!("compiled", &name); | ||
|
|
||
| let result = | ||
| unsafe { compiler.jit(&name, &code, spec_id) }.expect("catastrophe"); |
There was a problem hiding this comment.
I'm a little confused as to the relationship between a compiler, a module, and a contract.
Should I be compiling all the code with the same compiler?
crates/node/src/compiler.rs
Outdated
|
|
||
| cache.lock().unwrap().insert(hash, result); | ||
|
|
||
| unsafe { compiler.clear().expect("could not clear") }; |
There was a problem hiding this comment.
I'm pretty sure this is wrong, as the functions that I'm storing in the hashmap are just linked back to this compiler. When I call clear and try to call the cached function, it's a use-after-free and I segfault.
I'm obviously thinking about using the compiler the wrong way, how should I be thinking about it?
I've read over your code here, but I believe that's for aot compilation instead of jit since you're writing everything out to files.
Should I just be creating a new compiler every time and storing the compiler as well?
There was a problem hiding this comment.
Should I just be creating a new compiler every time and storing the compiler as well?
Yes that's right
crates/node/src/compiler.rs
Outdated
| let cache = cache.clone(); | ||
|
|
||
| move || { | ||
| let context = Box::leak(Box::new(revmc::llvm::Context::create())); |
There was a problem hiding this comment.
You should be able to use with_llvm_context and nest the implementation inside of the closure
crates/node/src/compiler.rs
Outdated
|
|
||
| cache.lock().unwrap().insert(hash, result); | ||
|
|
||
| unsafe { compiler.clear().expect("could not clear") }; |
There was a problem hiding this comment.
Should I just be creating a new compiler every time and storing the compiler as well?
Yes that's right
crates/node/src/evm.rs
Outdated
| // TODO: not sure if this is correct, I don't see a call-site for this method | ||
| let spec_id = revm_spec(chain_spec, &Head::default()); | ||
|
|
||
| compiler::ExternalContext::new(spec_id) |
There was a problem hiding this comment.
spec ID should also be dynamic and received from the channel
I thought that the context was created once per config, but that was not the case. This commit moves the compiler thread initialization to the config initialization and it all seems to work.
Closes #25
Will add more of a description later