feat: optimize plugin ram usage - #430
Conversation
Greptile SummaryThe PR reduces plugin memory usage by making TinyGo the preferred local build toolchain, renaming the host-facing allocator export, and lowering the default plugin payload limit.
Confidence Score: 3/5The PR should not merge until compatibility with existing plugin allocator exports and the effective HTTP payload-limit mismatch are addressed. Existing WASM plugins can load but fail on first memory write after the allocator rename, while near-limit HTTP bodies accepted by the edge can be rejected later as internal plugin errors after JSON encoding. Files Needing Attention: services/api/internal/platform/plugin/runtime.go, services/api/internal/config/load.go, scripts/README.md
|
| Filename | Overview |
|---|---|
| services/api/internal/platform/plugin/runtime.go | Renames the allocator ABI and lowers the envelope/event limit; lacks compatibility with existing malloc artifacts and rejects some bodies already accepted by the HTTP edge. |
| services/api/internal/config/load.go | Changes the operational default payload limit to 1 MiB, exposing the encoded-envelope limit mismatch. |
| scripts/install-local-plugin.sh | Adds validated TinyGo/Go selection and a clear fallback, but makes TinyGo the default prerequisite. |
| scripts/README.md | Describes TinyGo in the workflow but leaves the requirements section inconsistent with the new default. |
| docs/plugins/developer-guide.md | Documents TinyGo reactor builds, allocator considerations, and standard-Go fallback. |
| services/api/internal/platform/plugin/testdata/poisonplugin/main.go | Updates the test fixture allocator export to match the new runtime ABI. |
Sequence Diagram
sequenceDiagram
participant Client
participant HTTP as Plugin HTTP handler
participant Runtime
participant WASM as Plugin WASM
Client->>HTTP: "Raw body <= 1 MiB"
HTTP->>HTTP: "Marshal request envelope<br/>(body becomes base64)"
HTTP->>Runtime: "HandleRequest(envelope > 1 MiB)"
Runtime-->>HTTP: Payload exceeds runtime limit
HTTP-->>Client: 500 plugin execution error
Comments Outside Diff (1)
-
scripts/README.md, line 160 (link)List TinyGo as required
The installer now defaults to TinyGo and exits when it is unavailable, but this requirements section still names Go as the backend compiler. Developers following the documented setup therefore encounter a missing-tool failure on the default installation command.
Reviews (1): Last reviewed commit: "feat: optimize plugin ram usage" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| malloc := m.ExportedFunction("paca_malloc") | ||
| if malloc == nil { | ||
| return nil, fmt.Errorf("plugin: malloc not exported") | ||
| return nil, fmt.Errorf("plugin: paca_malloc not exported") | ||
| } |
There was a problem hiding this comment.
When an installed plugin was built against the previous allocator ABI, resolving only paca_malloc lets it load but causes every request, event, or host-result memory write to fail with plugin: paca_malloc not exported until the artifact is rebuilt.
| malloc := m.ExportedFunction("paca_malloc") | |
| if malloc == nil { | |
| return nil, fmt.Errorf("plugin: malloc not exported") | |
| return nil, fmt.Errorf("plugin: paca_malloc not exported") | |
| } | |
| malloc := m.ExportedFunction("paca_malloc") | |
| if malloc == nil { | |
| // Preserve compatibility with plugins built against the previous allocator ABI. | |
| malloc = m.ExportedFunction("malloc") | |
| } | |
| if malloc == nil { | |
| return nil, fmt.Errorf("plugin: paca_malloc or malloc not exported") | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| MaxMemoryPages: 1024, // 64 MiB | ||
| MaxRequestBodyBytes: 10 * 1024 * 1024, // 10 MiB | ||
| MaxMemoryPages: 1024, // 64 MiB | ||
| MaxRequestBodyBytes: 1 * 1024 * 1024, // 1 MiB — keep in sync with plugin-sdk-go's mallocBuffer size |
There was a problem hiding this comment.
When a raw plugin request is below 1 MiB but its JSON envelope exceeds 1 MiB after the []byte body is base64-encoded, the HTTP boundary accepts it and callExport rejects it as an internal plugin error. This reduces the effective raw-body ceiling to roughly 750 KiB and returns a 500 instead of a consistent payload-too-large response.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Two things worth addressing before merge: a new allocator-contract bug in writeToMemory, and a backward-compatibility break for already-installed plugin binaries.
Reviewed changes
This review covers the single commit e89cecc (13 files, +103/−20), which reframes plugin builds around TinyGo and tightens the host's memory accounting:
- TinyGo as the default plugin toolchain —
developer-guide.md/sdk-reference.mdand the scripts docs now prescribetinygo build -target=wasip1 -buildmode=c-shared, andinstall-local-plugin.shadds a--go-toolchain/GO_TOOLCHAINselector (defaulttinygo) with toolchain-presence validation. - Host allocator export renamed
malloc→paca_malloc—writeToMemorynow requires the plugin to exportpaca_malloc(the old name collides with themalloc/freeTinyGo's wasi-libc bundles), and thecacheplugin/poisonplugin/echopluginfixtures were renamed in lockstep; this matches plugin-sdk-gomaster, which I verified exportspaca_malloc/paca_freeover a fixed 1 MiBmallocBufferarena. - Lowered default request-body ceiling —
PLUGINS_MAX_REQUEST_BODY_BYTESdrops 10 MiB → 1 MiB inload.go,.env.example, and the defaults test, sized to match the SDK's 1 MiB arena.
⚠️ Existing plugin binaries break when this deploys
writeToMemory now requires a paca_malloc export and no longer falls back to the plugin's malloc. Every already-installed plugin binary compiled against the previous SDK exports only malloc, so after this ships, every host function that returns data into plugin memory (cache_get, storage_get, db_query2's error path, db_exec) fails inside writeToMemory — often silently, since several callers ignore the returned error (runtime.go:801, and the known storage_get/cache_get panic-prone pattern). The PR updates the SDK contract, but ships no migration story: no minimum-core-version gate (the mechanism the repo already has), no fallback read path, and no rebuild-required note.
Technical details
# Preserve compatibility with already-compiled plugin binaries
## Affected sites
- services/api/internal/platform/plugin/runtime.go:2203-2205 — the export lookup switched from "malloc" to "paca_malloc" with no fallback
## Required outcome
- Deploying this API must not silently break plugins that were compiled before the SDK's paca_malloc rename.
## Suggested approach
- In `writeToMemory`, resolve `paca_malloc`, and if absent fall back to `malloc` (old-SDK binaries keep working because their own `wasmSlice` resolves pointers relative to their own arena; new-SDK binaries use the primary path). Confirm plugin-sdk-go's change is tagged/released (not just `master`) so plugin authors building against the new docs pin a stable version. Consider bumping the core-version check so genuinely old plugins fail loudly at load rather than via ignored `writeToMemory` errors.ℹ️ 1 MiB default not propagated to production example and docs
load.go and services/api/.env.example now default PLUGINS_MAX_REQUEST_BODY_BYTES to 1048576, but deploy/.env.production.example:151 still pins 10485760 and docs/plugins/backend-plugin-system.md:282 still documents 10 MiB as default — so production installs following the example keep the old 10 MiB cap (the RAM win this PR targets won't materialize), and the docs table is now wrong. Align the two files with whichever value is intended; if the prod example stays at 10 MiB deliberately, the doc table still needs correcting.
ℹ️ Nitpicks
scripts/install-local-plugin.sh:87— the newGO_TOOLCHAINvariable collides with Go's own toolchain-selector env var (Go 1.21+ usesGO_TOOLCHAIN=auto|local|go1.x.y); a user with that set will be rejected by the new validation at line 160 with a confusing error.PLUGIN_GO_TOOLCHAINwould avoid the collision.- The new docs claim "Standard Go 1.21+", but plugin-sdk-go's README says standard Go 1.24+ works; the version claim should match the SDK so users don't pin a Go release that won't build.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| malloc := m.ExportedFunction("paca_malloc") | ||
| if malloc == nil { | ||
| return nil, fmt.Errorf("plugin: malloc not exported") | ||
| return nil, fmt.Errorf("plugin: paca_malloc not exported") |
There was a problem hiding this comment.
When the plugin's allocator can't fit the payload, plugin-sdk-go's wasmMalloc returns 0. writeToMemory doesn't check that before m.Memory().Write(uint32(ptr), data), so the payload is written into linear memory at offset 0 — corrupting the module's low memory (including the SDK's cached mallocBase global) — and it returns [0, len], which the SDK's wasmSlice maps to nil. The fixed 1 MiB arena makes this reachable in the real SDK path (a near-1 MiB request body followed by any host result written into the same arena), whereas the previous allocator would have grown memory.
Technical details
# writeToMemory must reject a 0 pointer from paca_malloc
## Affected sites
- services/api/internal/platform/plugin/runtime.go:2207-2215 — `results[0]` is never checked for 0 before `m.Memory().Write(uint32(ptr), data)` writes guest memory at offset 0 and the `[0, len]` pair is returned
## Required outcome
- When the guest allocator cannot fit the payload, `writeToMemory` must return an error instead of writing to linear-memory offset 0 and surfacing a nil-mapped pointer.
## Suggested approach
- In `writeToMemory`, check `results[0] == 0` after the call and return `fmt.Errorf("plugin: malloc returned 0 (buffer exhausted)")` before writing. Callers that already ignore the error (`db_exec`, `storage_get`, `cache_get`) degrade to their existing empty-result paths instead of corrupting memory.1 participant
Summary
Cuts plugin memory overhead by switching the recommended/default plugin build toolchain from standard Go to TinyGo, and tightens the request body limit to match.
install-local-plugin.shnow defaults to TinyGo (--go-toolchain/GO_TOOLCHAINlets you opt back into standard Go for plugins that hit a TinyGo stdlib limitation, e.g.reflect-heavy code likehtml/template).-buildmode=c-sharedfor TinyGo builds. Without it, TinyGo skips wiring up the WASI reactor entry point (_initialize), and every call into the plugin panics at runtime with"//go:wasmexport function called before runtime initialization". Docs and scripts are updated accordingly.malloctopaca_malloc. TinyGo-built plugins already export their ownmalloc/freefrom wasi-libc, which collided withplugin-sdk-go's export of the same name.runtime.go'swriteToMemoryand all WASM test fixtures (cacheplugin,poisonplugin,echoplugin) are updated to usepaca_malloc.PLUGINS_MAX_REQUEST_BODY_BYTESdefault from 10 MiB to 1 MiB, matchingplugin-sdk-go'smallocBuffersize, to avoid over-allocating per-call buffers.//go:wasmimporthost-function calling convention difference: TinyGo requires calling these by name directly rather than passing them as function values (e.g. viaplugin.CallHostFunction), since it doesn't support taking their address.Why
Every loaded plugin previously paid for its own copy of the full Go runtime, which becomes expensive in aggregate as more plugins are installed. Building with TinyGo by default (with standard Go as an explicit opt-out) reduces per-plugin host memory footprint significantly.
Test plan
go test ./services/api/internal/config/...(defaultMaxRequestBodyBytesupdated to 1 MiB)go test ./services/api/internal/platform/plugin/...(test fixtures rebuilt withpaca_mallocexport)go test ./services/api/test/e2e/...tinygo build -target=wasip1 -buildmode=c-sharedand confirm it loads and responds correctly