From 36ec2f97fffcc341aa32522bf48e64d8b041ba80 Mon Sep 17 00:00:00 2001 From: Adam Wick Date: Thu, 20 Jun 2024 14:21:26 -0700 Subject: [PATCH] =?UTF-8?q?=E2=81=89=EF=B8=8F=20Switch=20to=20using=20the?= =?UTF-8?q?=20on-demand=20allocator,=20instead=20of=20the=20pooling=20allo?= =?UTF-8?q?cator=20(#391)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #255. In addition to just fixing #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. --- lib/src/execute.rs | 40 ++---------------------- lib/src/linking.rs | 76 ++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 65 insertions(+), 51 deletions(-) diff --git a/lib/src/execute.rs b/lib/src/execute.rs index 4057f617..fd490953 100644 --- a/lib/src/execute.rs +++ b/lib/src/execute.rs @@ -675,9 +675,7 @@ fn configure_wasmtime( allow_components: bool, profiling_strategy: ProfilingStrategy, ) -> wasmtime::Config { - use wasmtime::{ - Config, InstanceAllocationStrategy, PoolingAllocationConfig, WasmBacktraceDetails, - }; + use wasmtime::{Config, InstanceAllocationStrategy, WasmBacktraceDetails}; let mut config = Config::new(); config.debug_info(false); // Keep this disabled - wasmtime will hang if enabled @@ -686,41 +684,7 @@ fn configure_wasmtime( config.epoch_interruption(true); config.profiler(profiling_strategy); - const MB: usize = 1 << 20; - let mut pooling_allocation_config = PoolingAllocationConfig::default(); - - // This number matches Compute production - pooling_allocation_config.max_core_instance_size(MB); - - // Core wasm programs have 1 memory - pooling_allocation_config.total_memories(100); - pooling_allocation_config.max_memories_per_module(1); - - // allow for up to 128MiB of linear memory. Wasm pages are 64k - pooling_allocation_config.memory_pages(128 * (MB as u64) / (64 * 1024)); - - // Core wasm programs have 1 table - pooling_allocation_config.max_tables_per_module(1); - - // Some applications create a large number of functions, in particular - // when compiled in debug mode or applications written in swift. Every - // function can end up in the table - pooling_allocation_config.table_elements(98765); - - // Maximum number of slots in the pooling allocator to keep "warm", or those - // to keep around to possibly satisfy an affine allocation request or an - // instantiation of a module previously instantiated within the pool. - pooling_allocation_config.max_unused_warm_slots(10); - - // Use a large pool, but one smaller than the default of 1000 to avoid runnign out of virtual - // memory space if multiple engines are spun up in a single process. We'll likely want to move - // to the on-demand allocator eventually for most purposes; see - // https://github.com/fastly/Viceroy/issues/255 - pooling_allocation_config.total_core_instances(100); - - config.allocation_strategy(InstanceAllocationStrategy::Pooling( - pooling_allocation_config, - )); + config.allocation_strategy(InstanceAllocationStrategy::OnDemand); if allow_components { config.wasm_component_model(true); diff --git a/lib/src/linking.rs b/lib/src/linking.rs index 24675aac..2e65edfe 100644 --- a/lib/src/linking.rs +++ b/lib/src/linking.rs @@ -6,15 +6,37 @@ use { wiggle_abi, Error, }, std::collections::HashSet, - wasmtime::{GuestProfiler, Linker, Store, UpdateDeadline}, + wasmtime::{GuestProfiler, Linker, Store, StoreLimits, StoreLimitsBuilder, UpdateDeadline}, wasmtime_wasi::{preview1::WasiP1Ctx, WasiCtxBuilder}, wasmtime_wasi_nn::WasiNnCtx, }; -#[derive(Default)] pub struct Limiter { /// Total memory allocated so far. pub memory_allocated: usize, + /// The internal limiter we use to actually answer calls + internal: StoreLimits, +} + +impl Default for Limiter { + fn default() -> Self { + Limiter::new(1, 1) + } +} + +impl Limiter { + fn new(max_instances: usize, max_tables: usize) -> Self { + Limiter { + memory_allocated: 0, + internal: StoreLimitsBuilder::new() + .instances(max_instances) + .memories(1) + .memory_size(128 * 1024 * 1024) + .table_elements(98765) + .tables(max_tables) + .build(), + } + } } impl wasmtime::ResourceLimiter for Limiter { @@ -22,22 +44,50 @@ impl wasmtime::ResourceLimiter for Limiter { &mut self, current: usize, desired: usize, - _maximum: Option, + maximum: Option, ) -> anyhow::Result { - // Track the diff in memory allocated over time. As each instance will start with 0 and - // gradually resize, this will track the total allocations throughout the lifetime of the - // instance. - self.memory_allocated += desired - current; - Ok(true) + // limit the amount of memory that an instance can use to (roughly) 128MB, erring on + // the side of letting things run that might get killed on Compute, because we are not + // tracking some runtime factors in this count. + let result = self.internal.memory_growing(current, desired, maximum); + + if matches!(result, Ok(true)) { + // Track the diff in memory allocated over time. As each instance will start with 0 and + // gradually resize, this will track the total allocations throughout the lifetime of the + // instance. + self.memory_allocated += desired - current; + } + + result } fn table_growing( &mut self, - _current: u32, - _desired: u32, - _maximum: Option, + current: u32, + desired: u32, + maximum: Option, ) -> anyhow::Result { - Ok(true) + self.internal.table_growing(current, desired, maximum) + } + + fn memory_grow_failed(&mut self, error: anyhow::Error) -> anyhow::Result<()> { + self.internal.memory_grow_failed(error) + } + + fn table_grow_failed(&mut self, error: anyhow::Error) -> anyhow::Result<()> { + self.internal.table_grow_failed(error) + } + + fn instances(&self) -> usize { + self.internal.instances() + } + + fn tables(&self) -> usize { + self.internal.tables() + } + + fn memories(&self) -> usize { + self.internal.memories() } } @@ -90,7 +140,7 @@ impl ComponentCtx { wasi: builder.build(), session, guest_profiler: guest_profiler.map(Box::new), - limiter: Limiter::default(), + limiter: Limiter::new(100, 100), }; let mut store = Store::new(ctx.engine(), wasm_ctx); store.set_epoch_deadline(1);