Repository navigation
perf(core): emit Config::clone once, box cross-crate async entry points, ICF on Linux release - #7050
Merged
Merged
Conversation
Added a Clone implementation for the config schema types so they can be duplicated when needed elsewhere in the core. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Added a Clone implementation for the config schema types so they can be duplicated when needed elsewhere in the core. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Added Clone implementations for the configuration schema types so they can be duplicated when needed elsewhere in the core. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce typed schema definitions for the configuration so values can be validated and described consistently. This lays the groundwork for surfacing config errors earlier and generating documentation from the schema. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a CoreContext helper that borrows the current embedder config and hands a reference to a closure, then switch the workspace snapshot, gated-service check, and API-key probe to use it. These call sites only needed a single field or predicate, so the previous full Config clone was wasted work on every dispatch. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…omorphization Wrap active_workspace_snapshot, agent_chat_for, and start_chat in #[inline(never)] functions returning BoxFuture, delegating to private async inner functions. Callers in openhuman-rpc and openhuman-embed previously re-instantiated each async body inside their own state machines, so boxing keeps a single compiled copy in this crate. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Release build guidance now records that 16 codegen units are used instead of one, since a single unit added 82% build time (tinyhumansai#5595). Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Document the project's conventions and working expectations in AGENTS.md so automated agents and contributors have a single reference for how changes should be made. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Install mold in the Linux build dependencies and set per-target RUSTFLAGS to link with mold and fold byte-identical functions via --icf=safe. The release profile's codegen units leave many duplicate functions that MSVC and Apple's linker already deduplicate but GNU ld does not, so this shrinks the stripped binary by roughly 2 MiB. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ests.rs Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Added Clone implementations for the config schema types so they can be duplicated when needed. This makes it possible to snapshot or pass around configuration values without moving them. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The manual Clone implementation for Config called clone on fields that are already Copy, so those calls were replaced with direct copies. This removes needless work and quiets clippy's clone-on-copy lint without changing any behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (13)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughThe PR updates Linux desktop linker settings and release-build guidance. It also changes core configuration access and cloning, and updates several public async functions to return boxed futures. ChangesLinux Desktop Build
Core Configuration and Async APIs
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to The configuration and chat changes have no identified behavior regression, and the Linux linker change has no identified build failure. Merge after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The inspected changes preserve existing authentication checks, embedder configuration authority, and protection of operator-wide active-user state. No introduced security concern was established. Credential persistence and Linux linker compatibility were not fully validated, so the assessment remains qualified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
I’m a rabbit with a linker tune, Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
12 tasks done
senamakel
pushed a commit
that referenced
this pull request
Oct 7, 2026
- Config::clone lists voice_live: #7050's hand-written clone and #7046's new field merged separately and left main failing to compile. - Conversations sends leave the model to the core by default again (composerModelOverride ?? undefined), as #6996 intended; #7048 had put hint:chat back while building on the damaged main, which also left the model-clear barrier unreachable. - Japanese gets the 109 strings added while it was missing; Turkish gets those plus the 61 restored English keys, and drops six memory-engine keys #7029 removed. - The legacy memory migration test checks that the sqlite backend key is gone, not that no key in the whole config contains "backend".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Clone for Configmarked#[inline(never)].#[derive(Clone)]marksclone#[inline], so the ~48 KiB function was emitted 18 times in a releaseopenhuman-core; it is now emitted once. Every field is listed in a struct literal, so a newConfigfield that is missing here is a compile error.active_workspace_snapshot,start_chatandagent_chat_fornow return a boxed future from an#[inline(never)]fn. Other crates (openhuman-rpc,openhuman-embed) await them, and anasync fnbody was being re-instantiated inside each calling crate. Call sites are unchanged (.awaitstill works).CoreContext::with_current_embedder_config(|c| …)reads the embedder config without cloning it. Three read-only callers used to clone a wholeConfigfor one field or anis_some()check:active_workspace_snapshot,ambient_config_has_api_keyandis_embedder_host.--icf=safe(fold byte-identical functions). MSVC already does this by default (/OPT:ICF) and Apple's ld deduplicates; GNU ld cannot.AGENTS.mdsaid release builds use one codegen unit; they use 16 (see 6941b18).Problem
A build-size audit (cold release build, product features,
cargo bloat) found that the shipped binary's size is dominated by first-party code, not dependencies. A large share of it is the same function emitted many times:#[inline]derives copied into each of the 16 codegen units, and async bodies re-instantiated in every crate that awaits them.Solution / measurements
Release
openhuman-core, product features, Linux x86_64, baseba1e9e6b4evs this branch before the upstream merge:--icf=safe.textPer function (
cargo bloat):<Config as Clone>::clone: 18 copies, 783 KiB → 1 copy.active_workspace_snapshot: 66 KiB → 10 KiB.start_chat: 142 KiB → 49 KiB.--icf=allwould save a little more (96.47 MiB), but it can fold functions whose addresses are compared, so this PR usessafe. Both theallandsafebinaries startedserveand answered/healthandopenhuman.config_getin a smoke test.Submission Checklist
config_clone_tests.rs(field-for-field equality via serialization, plus independence of the clone) andthe_current_embedder_config_can_be_read_without_cloning_it(scoped config and the no-config case).Impact
moldis installed in the build-desktop job (apt). The per-target rustflags env vars reach only the--targetbuild, not build scripts.Related
relaxed_jsonreplaced by tinytools-agent.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
build-size-auditValidation Run
pnpm --filter openhuman-app format:check: N/A, no frontend changes.pnpm typecheck: N/A.cargo test -p openhuman --libwith product features gave 8374 passed, 2 failed. The two failures (backend_dispatch_forwards_the_workflow_connection_idandpersisted_mixed_case_cloud_default_uses_configured_slug_after_restart) pass when run alone, on this branch and on the base, so they are parallel-test races on shared global state.rustfmton the changed files;cargo clippy -p openhuman --testswith product features is clean.openhuman-cli(which pulls in rpc, embed and tinyhumans) built in release. After merging upstream,openhuman-embedfails to compile on upstreammainitself (previous_session_store, from da166ad / Host-injected session store: per-agent conversations outside the workspace #7044); this PR touches no embed files.pnpm rust:layoutfails onagent/session_host/builder/factory.rs(1003 > 1001 lines). That file is identical to upstreammain.Summary by CodeRabbit