runtime: avoid heap allocs for plain errors#5480
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates TinyGo’s runtime panic paths to use preallocated runtime.Error values (via *plainError) to avoid allocations during runtime panics, and adds a build test intended to catch allocation-related failures when -gc=none.
Changes:
- Introduces preallocated
plainErrorinstances and changes runtime panics to pass*plainErrorinstead of constructing errors from strings at the panic site. - Updates several runtime panic call sites (Unix/Windows signals, channels, GC blocks, interface/hashmap) to use the preboxed errors.
- Adds a compiler/build regression test that builds a trivial panic program with
GC=none/Scheduler=none.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/runtime/runtime_windows.go | Switches some exception-triggered panics to use preboxed runtime errors. |
| src/runtime/runtime_unix.go | Switches signal/unsupported-signal panics to use preboxed runtime errors. |
| src/runtime/panic.go | Changes runtime panic plumbing to accept/store *plainError and adds runtimePanicAtMsg. |
| src/runtime/interface.go | Replaces string-based runtime panics with preboxed runtime errors. |
| src/runtime/hashmap.go | Replaces string-based runtime panics with a preboxed runtime error. |
| src/runtime/gc_blocks.go | Replaces string-based runtime panics with preboxed runtime errors for alloc/OOM paths. |
| src/runtime/error.go | Changes plainError to pointer receiver and defines the preboxed error set. |
| src/runtime/chan.go | Replaces string-based runtime panics with preboxed runtime errors for channel misuse. |
| main_test.go | Adds a build-only regression test for gc=none with a trivial nil-deref panic program. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Cause a runtime panic, which is (currently) always a string. | ||
| func runtimePanic(msg string) { | ||
| // As long as this function is inined, llvm.returnaddress(0) will return | ||
| // As long as this function is inlined, llvm.returnaddress(0) will return | ||
| // something sensible. | ||
| runtimePanicAt(returnAddress(0), msg) | ||
| runtimePanicAtMsg(returnAddress(0), &errRuntime, msg) | ||
| } |
There was a problem hiding this comment.
There is a more invasive change that could do this, not sure how people would like it, though 😄
jakebailey
changed the title
jakebailey
force-pushed
the
5438-followup
branch
from
929295f to
7e5c7ef
Compare
| func runtimePanicAt(addr unsafe.Pointer, msg string) { | ||
| runtimePanicAtMsg(addr, plainError(msg), msg) | ||
| } |
Lower constant-string calls to runtime.runtimePanic and runtime.runtimePanicAt so they pass a runtime.plainError interface value to runtimePanicAtMsg. This uses static interface backing storage for recoverable runtime panics instead of allocating a string header at run time. Keep dynamic runtimePanic(msg) as the exact fallback. It constructs the runtime.Error value only when a recover frame is present, so trap/abort paths do not eagerly box the string.
jakebailey
force-pushed
the
5438-followup
branch
from
7e5c7ef to
b95c9b7
Compare
|
Alternative idea: if the message to be stored in the interface is a single word (such as a pointer), it doesn't need an extra allocation to turn it into an interface. (I would prefer if we could avoid compiler changes for this optimization). |
|
Ultimately the problem is code which passes in arbitrary strings; there are places outside the runtime that linkname into this specific function and pass in their own strings. It's not clear to me how those callers would be able to do this besides maybe having an I think this optimization can actually be generalized, though, which could be an alternative too. |
This addresses #5438 (comment)
I don't think this will change memory usage; we already have the strings in the binary AFAIK, so this should be free. Though it does beg the question of why these are not memoized in the old code... (will look into that)