JIT rework, misc fixes #82
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Owner
qmonnet
commented
May 16, 2023
qmonnet
commented
- src/lib.rs,src/interpreter.rs: Move interpreter to dedicated module
- src/interpreter.rs: Update clippy linter names (cast_ptr_alignment)
- src: Fix rust-analyzer diagnostics for shorthand struct initialization
- src/insn_builder.rs: Remove unnecessary 'mut' reported by rust-analyzer
- src/{jit.rs,lib.rs}: Split struct JitMemory, make JITed prog an object
- src/jit.rs: Fix memory leak in JIT-compiler
- src/jit.rs: Move emitting functions to JitCompiler
- src/jit.rs: Move JitMemory under JitCompiler
- ci: Add Rust and Asan flags (memory leaks, address sanitation) for tests
- ci: Split workflow, avoid running conformance/coverage w/ all toolchains
The interpreter is rather self-contained, assuming we can pass it the program and the list of helpers to use. Let's move it to a dedicated module, to lighten lib.rs. No functional change. Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Clippy says the name of the linter is deprecated and should be updated. Let's update it now. Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
qmonnet
force-pushed
the
pr/misc-rework
branch
6 times, most recently
from
986a31e
to
3bb802a
Compare
Context for this commit: We have a memory leak in the JIT compiler, due to the fact that we never free the memory area used by the program. It is trivial to implement the Drop() trait for struct JitMemory, but this wouldn't work: this object is dropped as soon as we have compiled the program, when we want to free the memory area only when we no longer use the program (in other words, when the VM is destroyed). We need to hold a reference to the memory area in the VM. This requires reworking the JIT-compiler somewhat. In the process, we split struct JitMemory into two structs, one containing the reference and offset to the actual memory area and that we want to keep until the VM is destroyed, the other one, struct JitCompiler, containing the data that we only need during compilation and that we can drop after the program has been generated. Signed-off-by: Quentin Monnet <quentin@isovalent.com>
We have a memory leak in the JIT-compiler: we never free the memory area used to store the machine code for the program. Let's implement the Drop() trait for the struct JitMemory so we can clean after ourselves. Reported-by: Ben Kimock <kimockb@gmail.com> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
These functions belong to the JIT compiler and should be part of its implementation. Also, remove the [#inline] directives: I can't remember what they were here for in the first place, possibly just because uBPF would inline the equivalent functions. Leave it to the compiler to decide what to inline. Signed-off-by: Quentin Monnet <quentin@isovalent.com>
JitMemory relies on JitCompiler, let's define the latter first. This way we also have all exports at the end of the file. Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Pass some flags to test more aspects of rbpf in the CI. We cannot pass those flags via the actions-rs/cargo Action, so we drop it for that step. Actually, drop it everywhere, we can just as well run cargo directly without having to pull another Action. Suggested-by: Ben Kimock <kimockb@gmail.com> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
There is not point in running conformance tests and coverage checks with all three toolchain versions in use for the rest of the CI. Let's split into different jobs. Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.