Use the wasmtime on-demand allocator #255
Comments
acfoltzer
changed the title
run mode shouldn't use the wasmtime pooling allocator
acfoltzer
added a commit
that referenced
this issue
May 3, 2023
No more `TEST_LOCK`! The integration tests now take less than 5 seconds to execute on my workstation, vs 79 seconds previously. This required two main pieces of work. 💽 Run the Viceroy test server on an ephemeral port =================================================== The server spun up to run incoming requests through Wasm now is bound to an ephemeral port rather than being hardcoded to port 7878, and the test requests have their URLs modified to include the ephemeral port assigned to the test. This change technically means the `Test::against*()` methods can no longer be used to send requests to destinations other than the Viceroy that's been spun up for this test, but that's the only way they're currently used anyway. The main concrete change here for test authors is that you no longer specify `localhost:7878` in your test requests, you instead just provide a request path. 🤵 Redesign test backend servers to also use ephemeral ports ============================================================ This is a broader-reaching change in a few ways: - Backends now _must_ have a service function defined, or test setup will panic. This was already how the integration tests behaved semantically, but this does change some of the syntax around their definitions. - Test servers for backends are spun up once per `Test`, rather than per invocation of `Test::against*()`. The existing suite of test backends was already mostly stateless, so again not a huge impact on test authors. - A full URL can no longer be specified for backends. Instead, a path may be provided in order to exercise the backend path prefix behavior of Viceroy. - Tests that must know dynamically how to reach a backend server now must explicitly start the backend servers before getting a URI suitable for the purpose. This only shows up in dynamic backend tests. 🛍️ Grab bag =========== - The level of parallelism made possible by this change meant that I had to tweak the default number of instances in the Wasmtime pooling allocator down from its default of 1000. I don't expect a limit of 100 to cause anyone problems, but this also raised the possibility of switching away from the pooling allocator for most use cases. A followup issue is available at #255. - The additional parallelism also caused a massive CPU bottleneck when JIT compiling the test fixture Wasm modules. The main workspace now enables opt-level 1 for debug builds, substantially improving runtime performance of the tests without heavily impacting compile speed. - The `trap-test` was using a `#[path = "..."]` mechanism to reuse code from the integration test suite. I didn't know this was a mechanism that existed, and it was cool to learn about it. But it also was incompatible with `common` now having a child module, so I inlined the very small handful of definitions that test was reusing. - I added a couple extra functions to support cases where a test might want to define a backend config separately from its service function, for example if starting a `Test` configuration from a `fastly.toml` file. These currently go unused because none of the existing `fastly.toml`-using tests define any backends. - The new `TestBackends` interfaces are awfully panicky. I figure this is OK for test infrastructure, but am open to suggestions if folks feel this is unwise. I tried to at least document the various expectations, and make the panic messages relevant beyond just a line number. - The graceful shutdown logic is no longer strictly necessary unless we write enough tests with enough backends to run out of ports. But because it seems to work fine and is a tidy way to handle things, I left it in.
acfoltzer
added a commit
that referenced
this issue
May 4, 2023
No more `TEST_LOCK`! The integration tests now take less than 5 seconds to execute on my workstation, vs 79 seconds previously. This required two main pieces of work. 💽 Run the Viceroy test server on an ephemeral port =================================================== The server spun up to run incoming requests through Wasm now is bound to an ephemeral port rather than being hardcoded to port 7878, and the test requests have their URLs modified to include the ephemeral port assigned to the test. This change technically means the `Test::against*()` methods can no longer be used to send requests to destinations other than the Viceroy that's been spun up for this test, but that's the only way they're currently used anyway. The main concrete change here for test authors is that you no longer specify `localhost:7878` in your test requests, you instead just provide a request path. 🤵 Redesign test backend servers to also use ephemeral ports ============================================================ This is a broader-reaching change in a few ways: - Backends now _must_ have a service function defined, or test setup will panic. This was already how the integration tests behaved semantically, but this does change some of the syntax around their definitions. - Test servers for backends are spun up once per `Test`, rather than per invocation of `Test::against*()`. The existing suite of test backends was already mostly stateless, so again not a huge impact on test authors. - A full URL can no longer be specified for backends. Instead, a path may be provided in order to exercise the backend path prefix behavior of Viceroy. - Tests that must know dynamically how to reach a backend server now must explicitly start the backend servers before getting a URI suitable for the purpose. This only shows up in dynamic backend tests. 🛍️ Grab bag =========== - The level of parallelism made possible by this change meant that I had to tweak the default number of instances in the Wasmtime pooling allocator down from its default of 1000. I don't expect a limit of 100 to cause anyone problems, but this also raised the possibility of switching away from the pooling allocator for most use cases. A followup issue is available at #255. - The additional parallelism also caused a massive CPU bottleneck when JIT compiling the test fixture Wasm modules. The main workspace now enables opt-level 1 for debug builds, substantially improving runtime performance of the tests without heavily impacting compile speed. - The `trap-test` was using a `#[path = "..."]` mechanism to reuse code from the integration test suite. I didn't know this was a mechanism that existed, and it was cool to learn about it. But it also was incompatible with `common` now having a child module, so I inlined the very small handful of definitions that test was reusing. - I added a couple extra functions to support cases where a test might want to define a backend config separately from its service function, for example if starting a `Test` configuration from a `fastly.toml` file. These currently go unused because none of the existing `fastly.toml`-using tests define any backends. - The new `TestBackends` interfaces are awfully panicky. I figure this is OK for test infrastructure, but am open to suggestions if folks feel this is unwise. I tried to at least document the various expectations, and make the panic messages relevant beyond just a line number. - The graceful shutdown logic is no longer strictly necessary unless we write enough tests with enough backends to run out of ports. But because it seems to work fine and is a tidy way to handle things, I left it in.
GeeWee
pushed a commit
to GeeWee/Viceroy
that referenced
this issue
Jul 25, 2024
…locator (fastly#391) Fixes fastly#255. In addition to just fixing fastly#255, this also allows the test suite to be run in parallel, again. At least on my machines, running `make test` directly would cause memory failures. I'm guessing this was due to the pooling allocator's use of virtual memory. Switching to the on-demand allocator means that things run fine. This extends the built-in `Limiter` object to use a `StoreLimits` under the hood, as suggested in the issue. It then builds different versions of this limiter based on whether or not Viceroy is operating in a component or non-component context, because component-based systems require more instances and more tables than traditional ones.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
For
run: this execution mode is one-and-done and not meant to emulate heap limits and such from real C@E, this should use Wasmtime's default allocator. This will both lower the virtual memory footprint of executions and allow unit tests to use more than the C@E heap limit (currently 128MiB)For
serve:StoreLimitsBuildershould let us provide fidelity against the real platform limitsThe text was updated successfully, but these errors were encountered: