Stream: git-wasmtime

Topic: wasmtime / Issue #2518 Implement the pooling instance all...

Wasmtime GitHub notifications bot (Dec 17 2020 at 07:45):

peterhuene commented on Issue #2518:

Hmm, MADV_DONTNEED may not be implemented for aarch64. I'll look into this soon.

Wasmtime GitHub notifications bot (Dec 17 2020 at 07:46):

peterhuene edited a comment on Issue #2518:

Hmm, MADV_DONTNEED may not be implemented for aarch64. I'll look into the test failure soon.

Wasmtime GitHub notifications bot (Dec 17 2020 at 07:46):

peterhuene edited a comment on Issue #2518:

Hmm, MADV_DONTNEED may not be implemented for aarch64-linux. I'll look into the test failure soon.

Wasmtime GitHub notifications bot (Dec 17 2020 at 07:47):

github-actions[bot] commented on Issue #2518:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:wasm", "wasmtime:api"

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

• peterhuene: wasmtime:api

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

Learn more.
</details>

Wasmtime GitHub notifications bot (Dec 17 2020 at 08:04):

cfallin commented on Issue #2518:

Hmm, MADV_DONTNEED may not be implemented for aarch64-linux. I'll look into the test failure soon.

The aarch64 tests run on qemu, which has this lovely bit of implementation in its syscall implementation (link):

    case TARGET_NR_madvise:
        /* A straight passthrough may not be safe because qemu sometimes
           turns private file-backed mappings into anonymous mappings.
           This will break MADV_DONTNEED.
           This is a hint, so ignoring and returning success is ok.  */
        return 0;

At some point we'd ideally run our tests on real hardware, but for now I suspect the best option would just be to disable the unit test and anything that depends on the zeroing semantics. (Or maybe a feature flag to eplicitly memset/bzero instead? That's bad if it was only sparsely committed though...)

Wasmtime GitHub notifications bot (Dec 17 2020 at 08:04):

cfallin edited a comment on Issue #2518:

Hmm, MADV_DONTNEED may not be implemented for aarch64-linux. I'll look into the test failure soon.

The aarch64 tests run on qemu, which has this lovely bit of implementation in its syscall handling (link):

    case TARGET_NR_madvise:
        /* A straight passthrough may not be safe because qemu sometimes
           turns private file-backed mappings into anonymous mappings.
           This will break MADV_DONTNEED.
           This is a hint, so ignoring and returning success is ok.  */
        return 0;

At some point we'd ideally run our tests on real hardware, but for now I suspect the best option would just be to disable the unit test and anything that depends on the zeroing semantics. (Or maybe a feature flag to eplicitly memset/bzero instead? That's bad if it was only sparsely committed though...)

Wasmtime GitHub notifications bot (Dec 18 2020 at 16:20):

alexcrichton commented on Issue #2518:

We already have a different flag for running in qemu to reduce virtual memory usage overhead since QEMU behaves differently than native processes in that respect, so perhaps that could also be where we configure "hey wasmtime madvise doesn't work as expected"

Wasmtime GitHub notifications bot (Feb 05 2021 at 23:39):

github-actions[bot] commented on Issue #2518:

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:

• peterhuene: wasmtime:c-api

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

Learn more.
</details>

Wasmtime GitHub notifications bot (Mar 06 2021 at 07:08):

peterhuene commented on Issue #2518:

@alexcrichton I think all of the feedback has now been addressed. FYI: the commit you stopped your review at was "fix bad merge".

This is also running the wast tests with the pooling allocator (+uffd on linux).

My TODO list following this PR:

• Investigate fuzzing the pooling allocator
• Move the fiber creation into the allocators (#2708)

Wasmtime GitHub notifications bot (Mar 08 2021 at 18:04):

peterhuene commented on Issue #2518:

Regarding the page alignment for instances and tables: we don't need it for instances for the pooling allocator, but we do need it for tables because the pooling allocator "decommits" table memory by page and you don't want any page to have elements from multiple tables.

The reason instances are paged-aligned is that this is an artifact from when the pooling allocator had one giant mmap'd region and the memories/tables of the instance came next, so they needed page-alignment back then.

I'll fix the instance pool to not page-align the instances and instead align to Instance's alignment requirements in a follow-up PR.

Regarding malloc/free for instances and tables, I think there's no inherent reason why they can't be used, but part of the intention of this design was to preallocate both (a desirable thing to do for a service) and as Instance is a magical DST, managing one's own memory mapping is probably the easiest way to accomplish that.

Last updated: Nov 22 2024 at 16:03 UTC