Uh oh!
There was an error while loading. Please reload this page.
[native] Replace std::function with function pointers - #12548
Conversation
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: 3
New issues introduced by this change (3)
| Severity | Finding |
|---|---|
src/native/common/include/runtime-base/timing-internal.hh — 💡 Documentation — The new comment reads a bit awkwardly (“thread state through”). Consider… | |
src/native/common/runtime-base/timing-internal.cc — 💡 Maintainability — Consider declaring line_writer as LineWriter instead of auto so it’s… | |
src/native/clr/include/host/assembly-store.hh — 💡 Documentation — Since the new API accepts a raw pointer and the implementation is explicitly… |
What changed in this PR
This PR reduces C++ standard library surface area in the CoreCLR host by replacing the remaining std::function callback parameters with lower-overhead alternatives (function pointer + void* context, and const char*), helping the broader effort to remove libc++ from the CoreCLR host and reducing startup allocations / binary size.
Changes:
- Reworked
FastTiming::dumpto use a function pointer callback (LineWriter) plus an opaquecontextpointer instead ofstd::function. - Updated
AssemblyStore::configure_from_payloadto takeconst char *store_pathdirectly, eliminating a per-startupstd::stringallocation and simplifying diagnostics. - Removed now-unneeded
<functional>includes from the affected headers.
| File | Description |
|---|---|
| src/native/common/runtime-base/timing-internal.cc | Switches FastTiming::dump callback invocation to (context, line); updates logcat/file dump call sites accordingly. |
| src/native/common/include/runtime-base/timing-internal.hh | Removes <functional>, introduces LineWriter typedef, updates dump signature. |
| src/native/clr/include/host/assembly-store.hh | Removes <functional>, updates configure_from_payload signature and comment. |
| src/native/clr/host/host.cc | Updates the single call site to pass store_path directly. |
| src/native/clr/host/assembly-store.cc | Replaces callback usage with optional_string (store_path) for null-safe diagnostics and debug logging. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
1f91b00 to
a35febfCompareeac3f03 to
84c4781Compare84c4781 to
bc86ae1Comparebc86ae1 to
3388358Compare3388358 to
352372dCompare352372d to
77419baCompare77419ba to
8df7b68Compare`std::function` is a type-erasing wrapper which needs to store, copy and destroy an arbitrary callable, and it pulls `<functional>` into every translation unit that sees the declaration. Neither of the two uses in the CoreCLR host needs any of that. `FastTiming::dump` took its line writer as `std::function<void(std::string_view const&)>` by value. Of its two callers one passes a captureless lambda and the other captures a single `FILE*`, so a plain function pointer plus an opaque `void *context` covers both: using LineWriter = void (*) (void *context, std::string_view const& line); `AssemblyStore::configure_from_payload` took a `const std::function<std::string()>&` used only to produce a path for diagnostics. Its only caller wrapped a `const char *` in a `std::string` just so that the callee could call `c_str ()` on it again, and the callback is invoked unconditionally in the success path, so this allocated a string on every startup. It now takes the `const char *` directly. This does not change the number of undefined libc++ references, since both uses were fully inlined by the optimizer, but it removes the generated machinery: `libnet-android.release.so` shrinks by 6,976 bytes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Both `dump` callers either write to a file or ignore the context entirely, so there is no need for the context to be `void*`. Typing it as `FILE*` removes the `static_cast` in the file line writer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
The two line writers were captureless lambdas converted to function pointers at the call site. That conversion goes through a compiler generated static invoker, so making them plain functions in an anonymous namespace removes a level of indirection: `libnet-android.release.so` shrinks by a further 56 bytes. The remaining lambdas inside `dump` are called directly rather than converted to function pointers, so the optimizer already inlines them completely - replacing those measured 2 bytes *larger*, so they are left alone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Addresses review feedback: `configure_from_payload()` takes a raw `const char*` and every use of it goes through `optional_string ()`, so the header comment now says explicitly that passing `nullptr` is allowed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
8df7b68 to
278ac99CompareUh oh!
There was an error while loading. Please reload this page.
simonrozsival
commented
Aug 28, 2026
Consolidated into #12545 to reduce the depth of the #12546 stack. No code changed: the commits from this PR are now part of #12545 unmodified, and the resulting tree is byte-identical. This PR sat directly on top of #12545 and touched the same files, so reviewing them together is easier than reviewing the same file across two intermediate states. |

Part of #12533. Stacked on top of #12547.
std::functionis a type-erasing wrapper: it has to be able to store, copy and destroy an arbitrary callable, and it pulls<functional>into every translation unit that sees the declaration. Neither of the two remaining uses in the CoreCLR host needs any of that.FastTiming::dumpThe line writer was taken by value:
Of its two callers,
dump_to_logcatpasses a captureless lambda anddump_to_filecaptures a singleFILE*. A plain function pointer plus an opaque context covers both:The context is typed as
FILE*rather thanvoid*, since the only caller that needs one writes to a file — that avoids a cast in the writer.dump_to_filepasses itsFILE*through instead of capturing it, anddumpstays out of line — no template, so no code duplication per callback type.The two writers are plain functions in an anonymous namespace rather than captureless lambdas. A lambda converted to a function pointer goes through a compiler generated static invoker, so using functions directly removes a level of indirection.
AssemblyStore::configure_from_payloadThe callback existed only to produce a store path for diagnostics, but the single caller was:
That wraps a
const char *in astd::stringpurely so that all three use sites can call.c_str ()on it again. One of those three is the success-pathlog_debugfat the end of the function, so the callback runs on every startup and this allocated a string every time.It now takes the
const char *directly and uses the existingoptional_string ()helper, which also makes it null-safe. The stale comment describing the callback was updated — it claimed the path was only used for invalid payloads, which was not true.Results
libnet-android.release.soThe undefined reference count is unchanged: both uses were fully inlined by the optimizer at
-O2, sostd::function's machinery was emitted into the objects rather than left as undefined references. What goes away is that generated machinery — 6,992 bytes come off the shared library overall (mostly.textand.bss), plus onestd::stringallocation per startup.Verification
format_managed_type_namewarning).llvm-nm --undefined-only/llvm-sizeover the same build tree, before and after.<functional>is no longer included bytiming-internal.hhorassembly-store.hh. The only remainingstd::functionreferences insrc/nativeare insrc/native/mono/, which this stack does not touch.On the unchanged reference count
Worth recording, since it is easy to misread as "no progress":
llvm-nm --undefined-onlylists each undefined symbol once per object file, so this metric counts (object, symbol) pairs rather than call sites.host.cc.ostill has 24 relocations againstoperator deleteand 8 against~basic_stringfrom its otherstd::stringuses, so dropping one use cannot remove the symbol from its undefined list. Measured per object across this change:.texthost.cc.oassembly-store.cc.oThe count only moves when the last use of a symbol in a given file goes away, so it behaves as a per-file cliff rather than a gradual measure.
The lambdas remaining inside
dumpare called directly instead of being converted to function pointers, so the optimizer already inlines them; replacing them with named functions measured 2 bytes larger, so they were left as they are.