Stream: git-wasmtime

Topic: wasmtime / PR #7854 Add C API for GuestProfiler

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 03:32):

Milek7 opened PR #7854 from Milek7:guestprofiler-c-api to bytecodealliance:main.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 03:32):

Milek7 requested alexcrichton for a review on PR #7854.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 03:32):

Milek7 requested wasmtime-core-reviewers for a review on PR #7854.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 03:37):

Milek7 commented on PR #7854:

Sorry for the noise with #7853, I messed up with Git.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 04:44):

github-actions[bot] commented on PR #7854:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 13:34):

alexcrichton submitted PR review:

Thanks for the PR here, and looks good to me! I've got some small suggestions here and there but let me know if I can clarify or help out.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 13:34):

alexcrichton submitted PR review:

Thanks for the PR here, and looks good to me! I've got some small suggestions here and there but let me know if I can clarify or help out.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 13:34):

alexcrichton created PR review comment:

A small suggestion might be to put modules_size after the modules_name and modules_module pointers since I think other parts of the C API typically do pointer-then-length. (but please correct me if I'm misremembering)

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 13:34):

alexcrichton created PR review comment:

Mind adding some documentation for this and the functions below? One thing I think is especially important to document is the ownership of pointers and who's responsible for deallocating what, for example.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 13:34):

alexcrichton created PR review comment:

This I think would best be modelled as profiler: Box<wasm_guestprofiler_t> rather than using MaybeUninit. That way this could be a safe function as well.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 17:22):

Milek7 updated PR #7854.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 17:30):

Milek7 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 17:30):

Milek7 created PR review comment:

Seems mixed, eg. wasm_byte_vec_new takes size first. I can change this but I feel that because this length applies to two following arguments it's clearer this way.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 17:32):

Milek7 updated PR #7854.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 17:39):

Milek7 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 17:39):

Milek7 created PR review comment:

On second though, it might be better to define struct {wasm_name_t*, wasmtime_module_t*} and pass pointer to that?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 17:39):

Milek7 edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 19:48):

Milek7 updated PR #7854.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 19:49):

Milek7 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 19:49):

Milek7 created PR review comment:

I changed to passing tuple as struct instead of column-major pointers.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 19:51):

Milek7 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 19:51):

Milek7 created PR review comment:

This is named mod instead of module because doxygen bugs out otherwise.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 19:54):

Milek7 updated PR #7854.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 19:59):

Milek7 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 19:59):

Milek7 created PR review comment:

Yep, I have no idea what I'm doing in Rust. It must be Box otherwise it would leave allocation which isn't really possible to clean up legally from C side.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 20:17):

Milek7 updated PR #7854.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 20:17):

Milek7 commented on PR #7854:

I think I addressed everything for now.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 03 2024 at 21:58):

Milek7 requested alexcrichton for a review on PR #7854.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2024 at 16:39):

alexcrichton submitted PR review:

Thanks for this!

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2024 at 17:05):

Milek7 updated PR #7854.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2024 at 17:31):

Milek7 requested alexcrichton for a review on PR #7854.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2024 at 18:38):

alexcrichton merged PR #7854.

Last updated: Nov 22 2024 at 16:03 UTC