jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
FWIW
jitdump
didn't actually require any extra C libraries or anything, so the ease of building it was why I went ahead and placed it in the default features. For the ittapi crate, however, it requires a native non-Rust toolchain, so we may want to leave this out of the default features for now while we sort out the build requirements there?
alexcrichton created PR Review Comment:
Is this perhaps stuck around from an old merge conflict?
alexcrichton created PR Review Comment:
FWIW idiomatically
use
statements all appear at the top of the module
alexcrichton created PR Review Comment:
How come this is cached internally? I don't think we publish code for the same module twice, so wouldn't we want to always create a new method id?
alexcrichton created PR Review Comment:
This
.unwrap()
is likely to panic since I don't think that all wasm modules have names associated with them
alexcrichton created PR Review Comment:
If this needs to be hooked up to get vtune working, I'd recommend implementing
Drop for VTuneAgent
which would call this method
alexcrichton created PR Review Comment:
Is the
Box
here necessary in thatijIT_NotifyEvent
is freeing the memory it's given? If the free isn't happening this may be a memory leak, but theBox
isn't necessary here either if all that's needed is a pointer. For that you can just do&mut jmethod as *mut _ as *mut _
alexcrichton created PR Review Comment:
This may be stray now at this point?
alexcrichton created PR Review Comment:
Should this be an argument to
module_load
perhaps? To try to get the source filename if possible?
alexcrichton created PR Review Comment:
er that is, an argument on the trait
jlb6740 submitted PR Review.
jlb6740 created PR Review Comment:
@jlb6740 it looks like the new crate here is pulling in bindgen at build time I think, right? That's a pretty hefty dependency and one we'd ideally rather avoid. Would it be possible to check in the generated bindings from bindgen via a scheme like pepyakin/binaryen-rs#52 perhaps?
Hi @alexcrichton, thanks for the comments. The profiling facilitated by a C library (https://github.com/intel/ittapi) for instrumentation and tracing and so there is a sys crate that uses bindgen to create the rust interface. Right now I think that crate isn't properly guarded on a windows build so that needs to be corrected, but it works well on linux. Yes .. we can make any update ... I am trying to digest the change here: https://github.com/pepyakin/binaryen-rs/pull/52/files. Did you remove the build time dependence on bindgen .. do it ahead of time and put the resulting file in bindings.rs? Is that what I am looking at there?
jlb6740 edited PR Review Comment.
jlb6740 edited PR Review Comment.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
@jlb6740 right yeah, I moved it to a
dev-dependencies
section so it's still verified that the bindings are correct oncargo test
, but otherwise the generated bindings are checked into the tree sobindgen
isn't needed at build time, only when testing the ittapi-rs crate
jlb6740 created PR Review Comment:
This is a good question. I saved this for supporting other calls that would rely on the same methodID. Right now this pr only calls method_load for a function and then moves on, but there are other events that can be recorded that aren't included with this particular patch (https://software.intel.com/en-us/vtune-help-ijit-notifyevent). Also .. I wasn't sure what the invoke flag was allowing now. In the past I ran an example wondering why my codes were running twice, only to realize I was using invoke when I did not need to. This was some action.rs file where I had extra hooks, which looks like it is gone now but I was calling profiling hooks twice when maybe it wasn't needed. I wanted to cache methodID to prepare for this possibility as well.
Also note, this instrumentation is targeting jitted code but future updates should include APIs for help in instrumenting wasmtime itself: https://software.intel.com/en-us/vtune-help-instrumenting-your-application#2CCDFEFA-30CD-4899-A86B-54C9F873C823. This is different from recording the methodID, but the same VTune agent interface here I expected would be used to cash domain ids and other handles that help with marking regions.
jlb6740 submitted PR Review.
jlb6740 submitted PR Review.
jlb6740 created PR Review Comment:
It doesn't at all ... it seems to only have purpose if you want to stop profiling and any point while code is still executing. Having drop for VTuneAgent call this would probably be a good idea nonetheless though. +1
jlb6740 edited PR Review Comment.
jlb6740 edited PR Review Comment.
jlb6740 edited PR Review Comment.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Ah ok caching makes sense, but I think that here it may make more sense to insert it into the map and assert that it was the first time we saw it? Modules should only be reigstered once and this would allow other VTune methods to get called as well.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 submitted PR Review.
jlb6740 created PR Review Comment:
Sorry .. missed this comment earlier. Yeah, that would be great. I looked at instantiate_module and I don't see anywhere the path to the file is being preserved. Is this something possible now?
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 submitted PR Review.
jlb6740 created PR Review Comment:
Great! Thanks. It shouldn't be freed. I think was some legacy when I didn't quite now how to represent this. Converts to pointer to jmethod and then coerces to the c_void type expected.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Yeah we don't actually really ever have a filename right now, but it seems like it'd be best if the caller of the profiler would be inventing a filename like this rather than relying on each profiler to invent its own filename. We could eventually thread through a filename from something like
Module::from_file
to here when we get around to binding it.
jlb6740 submitted PR Review.
jlb6740 created PR Review Comment:
Ok, make sense. I'll add the argument and make up the name at the call site. :+1:
jlb6740 submitted PR Review.
jlb6740 created PR Review Comment:
You know, I was thinking if filename should eventually come from the module, well the module is already passed in. We already pull the module_name (if exists) and method_name from the module and the only reason we don't pull the filename too is because it isn't there. I think ultimately we actually may not want this added to the interface. In the next push I've only added it to event load to see what you think, but if you still think it should be a part of the interface and passed in from the jit call site, then I can add it there too.
jlb6740 submitted PR Review.
jlb6740 created PR Review Comment:
:+1:
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
I think this may still need to be removed?
alexcrichton created PR Review Comment:
I think this may not exist in the C API yet? (should just be a few extra lines here and there for this PR)
jlb6740 submitted PR Review.
jlb6740 created PR Review Comment:
Lol .. Yes sorry. :+1: I was following your lead and didn't go back and add. I see support here should be added in crates/c-api and will do that next push.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
I think this may be missing from the commit by accident?
alexcrichton created PR Review Comment:
Oh dear I see what you mean, it looks like some mangling has definitely gone awry here by accident :(
jlb6740 submitted PR Review.
jlb6740 created PR Review Comment:
Let me just remove this.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
jlb6740 updated PR #819 from add-vtune-ittapi-support
to master
:
This patch adds initial support for ittapi which is an open
source profiling api for instrumentation and tracing and profiling
of jitted code.Build:
cargo build --features=profiling
Profile: // Using amplxe-cl from VTune
amplxe-cl -v -collect hostpost target/debug/wasmtime --jitprofile test.wasmNote, Vtune is a free open source profiling tool for identifying performance bottlenecks. This patch which brings in ittapi-rs when the profiling feature is enabled, currently only supports the profiling of jitted code but I am investigating using ittapi for profiling of wasmtime itself with a future patch that builds on this one.
alexcrichton merged PR #819.
Last updated: Nov 22 2024 at 17:03 UTC