[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. |
1f91b00 to
a35febf
Compare
eac3f03 to
84c4781
Compare
84c4781 to
bc86ae1
Compare
bc86ae1 to
3388358
Compare
3388358 to
352372d
Compare
352372d to
77419ba
Compare
77419ba to
8df7b68
Compare
`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
278ac99
Compare
|
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.