-
Notifications
You must be signed in to change notification settings - Fork 824
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
Metering middleware. #1839
Metering middleware. #1839
Changes from 11 commits
e7dd725
6537720
13a979a
9d8f315
17e7fea
77b99a0
89762a6
8ca3693
363a28c
970af1a
21369a2
05d8970
32b3dae
f110c4d
097bfeb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ use wasmer_compiler::wasmparser::BinaryReaderError; | |
use wasmer_compiler::TrapInformation; | ||
use wasmer_compiler::{Compilation, CompileError, CompiledFunction, Compiler, SectionIndex}; | ||
use wasmer_compiler::{ | ||
CompileModuleInfo, CompilerConfig, GenerateMiddlewareChain, MiddlewareBinaryReader, | ||
CompileModuleInfo, CompilerConfig, MiddlewareBinaryReader, ModuleMiddlewareChain, | ||
ModuleTranslationState, Target, | ||
}; | ||
use wasmer_compiler::{FunctionBody, FunctionBodyData}; | ||
|
@@ -47,16 +47,19 @@ impl Compiler for SinglepassCompiler { | |
fn compile_module( | ||
&self, | ||
_target: &Target, | ||
compile_info: &CompileModuleInfo, | ||
compile_info: &mut CompileModuleInfo, | ||
_module_translation: &ModuleTranslationState, | ||
function_body_inputs: PrimaryMap<LocalFunctionIndex, FunctionBodyData<'_>>, | ||
) -> Result<Compilation, CompileError> { | ||
if compile_info.features.multi_value { | ||
return Err(CompileError::UnsupportedFeature("multivalue".to_string())); | ||
} | ||
let vmoffsets = VMOffsets::new(8, &compile_info.module); | ||
let memory_styles = &compile_info.memory_styles; | ||
let table_styles = &compile_info.table_styles; | ||
let mut module = (*compile_info.module).clone(); | ||
self.config.middlewares.apply_on_module_info(&mut module); | ||
compile_info.module = Arc::new(module); | ||
Comment on lines
+59
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will leave all the old There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed that was a source of the bug fixed in 363a28c of this PR. I'm trusting that the tests will catch any real problems here. (Also, there are no old Arc's? At least not in this function?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I'm not sure about its correctness in context, but it's possible that this could cause problems later |
||
let vmoffsets = VMOffsets::new(8, &compile_info.module); | ||
let module = &compile_info.module; | ||
let import_trampolines: PrimaryMap<SectionIndex, _> = (0..module.num_imported_functions) | ||
.map(FunctionIndex::new) | ||
|
@@ -73,7 +76,10 @@ impl Compiler for SinglepassCompiler { | |
.collect::<Vec<(LocalFunctionIndex, &FunctionBodyData<'_>)>>() | ||
.par_iter() | ||
.map(|(i, input)| { | ||
let middleware_chain = self.config.middlewares.generate_middleware_chain(*i); | ||
let middleware_chain = self | ||
.config | ||
.middlewares | ||
.generate_function_middleware_chain(*i); | ||
let mut reader = | ||
MiddlewareBinaryReader::new_with_offset(input.data, input.module_offset); | ||
reader.set_middleware_chain(middleware_chain); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slight typo in the version, should drop the
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks.