Skip to content

Commit 6a96a72

Browse files
vorporealwarp-agentwarp-for-oss[bot]
authored
Compile time: share settings registration code across settings (#15454)
## Description This is PR 2 of 3 in a stack that reduces the compile time of the `warp` crate. It is stacked on #15453. The settings macros are the largest single source of macro-expanded code in the `warp` crate. Before this change, they produced 10.7 MB of the 36.7 MB total macro expansion, and the settings registration path produced about 660K lines of LLVM IR. This PR makes two changes: 1. **Move the registration body out of the macro.** The `register_settings_events!` macro expanded a full registration function (parse, update, clear, load, equality, and syncable callbacks) for every one of the ~300 registered settings. The body now lives in `settings::registration::register_setting_events_impl`, which is generic over only the settings group type and the value type. Settings that share those two types (for example, the many `bool` settings in one group) now share one compiled instantiation. A thin per-setting wrapper gathers the `Setting` trait metadata (storage key, defaults, platforms, and so on) into a plain struct before it hands off to the shared body. 2. **Emit change events through a trait.** `define_setting!` and `implement_setting_for_enum!` expanded four separate `concat_idents!` blocks per setting to construct the group's `*ChangedEvent` variant at each emit site. Each setting now gets one small `SettingChangeEvent` impl, and the emit sites call `<Self as SettingChangeEvent>::change_event(reason)`. This cuts `concat_idents!` from 606 uses (2.49 MB) to 351 uses (0.13 MB). ### How method resolution is preserved The old macro expansion called setting methods (for example `set_value` and `current_value_is_syncable`) at the expansion site. This matters: `Theme`, `SystemThemes`, and one test type define *inherent* `current_value_is_syncable` methods that shadow the `Setting` trait default. To keep this behavior, the macro builds a `SettingCallbacks` struct of function pointers at its expansion site. Each callback is a small non-capturing closure with the concrete setting type, so inherent methods still shadow trait defaults exactly as before. The shared generic body only invokes the function pointers. ### Alternatives considered and discarded - **Per-setting generic function (measured).** A first version kept the whole body generic over the setting type `S`. It removed the macro bytes but only cut 22K IR lines (−0.13%), because the body still monomorphized once per setting. The (group, value) split in this PR cuts the registration path from 660K to 238K IR lines. - **Trait default method on `Setting`.** Moving the body into a default method has the same per-setting monomorphization problem, and it changes method resolution for the shadowed inherent methods. - **Full type erasure with `Box<dyn Any>`.** This would compile the body exactly once, but it needs runtime downcasts on every settings update and loses type safety. The (group, value) fn-pointer design gets most of the win with no runtime cost. - **Group-level registration aggregation.** Registering a whole group in one call could remove more per-setting glue, but it is a larger behavioral change and risks the schema byte-identity guarantee. It can be a follow-up. ### Measured results (Apple M5 Pro, rustc 1.92.0, dev profile, relative to PR 1) - `cargo llvm-lines -p warp --lib`: 17,279,212 → 17,022,929 lines (−1.5%); copies 580,747 → 573,201. The settings registration path drops from ~660K to ~238K lines (−64%). - Macro expansion (`-Zmacro-stats`): 36.7 MB → 32.2 MB (−12.4% versus master). The 1.99 MB `generate_settings_event_fn!` output is gone. - Touch-incremental rebuild of the `warp` lib: unchanged (15.4/14.8 s versus 15.4/14.6 s). - Clean rebuild of the `warp` lib (deps cached): unchanged within run-to-run noise (71–74 s versus 68 s; master measured ~71 s on the same machine). The wall-clock effect of this PR alone is within measurement noise. The change still removes a large fixed cost from macro expansion and codegen, and it makes each new setting much cheaper to add. ## Linked Issue N/A — compile-time work from a profiling session. - [ ] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. - [ ] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). ## Testing - `cargo nextest run -p settings`: 71/71 passed. - `cargo nextest run -p warp -E 'test(cloud_preferences) or test(theme) or test(/settings::/)'`: 168/168 passed. - `generate_settings_schema` output (221 settings) is byte-identical to the output on the base branch. - `./script/format` and all presubmit clippy passes are clean. This is a pure refactor with no user-visible behavior change, so no new tests were added. The schema byte-identity check plus the existing settings test suite cover the behavior that could regress. - [ ] I have manually tested my changes locally with `./script/run` ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode [Warp conversation](https://staging.warp.dev/conversation/024e797d-1d35-4c62-8394-abf93f7ddb0e) Co-Authored-By: Warp <agent@warp.dev> CHANGELOG-NONE --------- Co-authored-by: Warp <agent@warp.dev> Co-authored-by: warp-for-oss[bot] <277970191+warp-for-oss[bot]@users.noreply.github.com>
1 parent 83b4c10 commit 6a96a72

3 files changed

Lines changed: 306 additions & 181 deletions

File tree

crates/settings/src/lib.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#[macro_use]
22
pub mod macros;
33
pub mod manager;
4+
pub mod registration;
45
pub mod schema;
56

67
// Re-export commonly used types and traits
@@ -640,6 +641,17 @@ pub trait Setting {
640641
fn is_value_explicitly_set(&self) -> bool;
641642
}
642643

644+
/// A trait that maps a setting to the change event of its settings group.
645+
///
646+
/// The setting macros implement this trait for each setting they define. This
647+
/// lets the shared `Setting` implementations construct the correct group event
648+
/// variant without expanding per-setting event code at each emit site.
649+
pub trait SettingChangeEvent: Setting {
650+
/// Returns the group event that reports a change to this setting for the
651+
/// given reason.
652+
fn change_event(reason: ChangeEventReason) -> <Self::Group as Entity>::Event;
653+
}
654+
643655
/// Shared persistence operations for typed settings backed by secure storage.
644656
///
645657
/// Implementors remain responsible for routing their [`Setting`] lifecycle

0 commit comments

Comments
 (0)