Skip to content

Reduce CFPreferences call spam by caching frequently-read preferences#638

Open
gastonmorixe wants to merge 7 commits into
gnachman:masterfrom
gastonmorixe:gaston/preference-caching
Open

Reduce CFPreferences call spam by caching frequently-read preferences#638
gastonmorixe wants to merge 7 commits into
gnachman:masterfrom
gastonmorixe:gaston/preference-caching

Conversation

@gastonmorixe
Copy link
Copy Markdown

@gastonmorixe gastonmorixe commented Apr 4, 2026

Note

Coded with Claude Opus 4.6 and Codex 5.4 xhigh

Summary

iTerm2 was hitting macOS preferences far too often for values that almost never change during a session.

From a 3-second log capture on a normal focused window, the app made 27,173 CFPreferences calls. Most of that traffic came from a small set of UI and rendering preferences being read again and again on hot paths.

This PR adds a cache for those reads inside iTermPreferences, so we stop going back through NSUserDefaults / CFPreferences for the same values over and over.

What we saw

Measured from the logs:

Line counts from the raw captures:

  • Public build: 27,175 lines
  • Patched build: 233 lines

Those files include the 2-line log header, so the actual CFPreferences event counts are:

  • Before: 27,173 calls in 3 seconds
  • After: 231 calls in 3 seconds

That is a 99.1% reduction.

Just as important, the iTerm-specific hot keys we were worried about dropped to zero in the patched capture. The remaining calls are almost entirely Apple/internal keys that iTerm2 does not control.

Root cause

The problem was not one expensive feature. It was repetition.

A handful of stable preferences were being read on hot paths like redraw, layout, and session/UI updates, with each read going all the way through NSUserDefaults and into CFPreferences. That meant:

  • a lot of useless work on the main thread
  • lock traffic inside the preferences stack
  • noise in the system log
  • extra context switching for values that were effectively constants until the user changed Preferences

The first cache pass fixed the performance side, but review found two correctness holes:

  1. A cold read could race with a write and put an older value back into the cache.
  2. A write from outside the process could change the defaults domain while the cache kept serving stale data.

This PR keeps the cache and fixes those edge cases.

What changed

  • Cache preference values in iTermPreferences so repeated reads stay in-process instead of going back to CFPreferences.
  • Track cache generations so an in-flight cold read cannot overwrite newer cache state after a concurrent write.
  • Invalidate the cache when the active defaults object actually changes, including out-of-process updates, instead of depending on broad notification behavior alone.
  • Keep explicit-set tracking fast, so callers can still tell “user explicitly set this” from “using the default” without bypassing the cache.
  • Add regression tests for the two correctness bugs:
    • a cold read must not overwrite a newer in-process write
    • an out-of-process defaults write must invalidate cached values
  • Keep the older Objective-C test coverage in sync, and update the legacy test target enough to build under the current Xcode setup.

Why this is safe

This does not add work to the common cache-hit path.

Once a value is cached, we still return it directly from memory. The extra logic only matters when:

  • a key is read for the first time after invalidation
  • a preference changes
  • a concurrent write or external update happens

So the hot path gets quieter, while the stale-value edge cases are fixed instead of papered over.

Validation

Targeted regression tests for the cache bugs now pass:

  • testColdReadDoesNotOverwriteNewerWrite
  • testExternalProcessWriteInvalidatesCache

Full ModernTests also ran after the fix. The new cache regressions stayed green, and the suite ended with the same small set of pre-existing failures outside this area.

Result

This turns a noisy, repeated stream of preference lookups into mostly one-time reads plus cache hits.

In the measured 3-second capture:

  • 27,173 CFPreferences calls became 231
  • the iTerm-specific hot preference lookups dropped to zero
  • the remaining calls are almost all outside the set of preferences this code manages

Fixed logs:

$ log stream --level debug --predicate 'eventMessage MATCHES[c] ".*(iTerm).*"'
Untitled

- Implement thread-safe caching in `iTermPreferences` for both raw and computed values to minimize system calls.
- Add throttling to `NSUserDefaultsDidChangeNotification` handling (max 1 update per 50ms) to prevent cache thrashing during rapid updates.
- Introduce `valueIsExplicitlySetForKey:` API to efficiently check for user-defined overrides without bypassing the cache.
- Add dedicated caching for `NoSyncEnableAutomaticProfileSwitchingLogging` in `iTermUserDefaults`.
- Update `PTYSession.m` to use the optimized `valueIsExplicitlySetForKey:` instead of direct `NSUserDefaults` access.
- Add `iTermPreferencesCachingTest` with comprehensive test coverage for threading, throttling, computed preferences, and explicit value checking.
- Merge two adjacent critical sections in objectForKey: into one
  atomic operation. The split allowed another thread to observe
  explicit-set tracking without the cached value, causing
  valueIsExplicitlySetForKey: to trigger a redundant lookup.

- Clear sPreferenceExplicitlySetKeys in setUserDefaultsOverrideForTesting:
  alongside the cache clear so tests start from a fully clean state.
- Move iTermPreferencesCachingTest.m from iTerm2XCTests/ to
  ModernTests/ so it runs under the ModernTests scheme.

- Promote sLastCacheClearMs to file scope and reset it in
  resetPreferenceCacheForTesting so the throttle doesn't persist
  across test cases.

- Fix test assertions that were too strict about exact NSUserDefaults
  call counts. FastAccessors observers and notification timing can
  cause additional reads; tests now verify caching behavior without
  assuming exact counts from setup/teardown side effects.

All 16 tests pass.
The ModernTests target contains only Swift tests. ObjC XCTest files
belong in iTerm2XCTests, which runs under the iTerm2Tests scheme.
- Replace blunt NSUserDefaultsDidChangeNotification with per-key KVO
  observation on the active NSUserDefaults instance. Cache only clears
  when an actual preference key changes, not on every defaults sync.

- Add cache generation counter to prevent a TOCTOU race where a
  concurrent cache clear between fetch and store could write stale
  data back into the cache.

- setWithoutSideEffectsObject: now clears the full cache before
  re-caching only the written key, which is correct for computed
  preferences that may depend on the changed key.

- setUserDefaultsOverrideForTesting: re-registers KVO observers on
  the new defaults object.

- Add iTermPreferencesConsistencyTests.swift with regression specs
  for cold-read-vs-write ordering and external-process invalidation.

- Expand iTermPreferencesCachingTest.m with KVO-specific tests.
- Add FRAMEWORK_SEARCH_PATHS for BetterFontPicker, ColorPicker, and
  SearchableComboListView to the iTerm2XCTests target, fixing the
  "@import BetterFontPicker" module-not-found build error.

- Update iTermRuleTest to use the current scoreForHostname: signature
  which now requires commandLine: and expressionValueProvider: params.

- Add missing return after XCTAssert(false) in iTermPasteHelperTest.
@gnachman
Copy link
Copy Markdown
Owner

gnachman commented Apr 5, 2026

Thanks for the detailed investigation and PR! I appreciate the effort to profile and reduce unnecessary work.

A few high-level concerns before getting into specifics:

NSUserDefaults performance in practice

The CFPreferences call counts from log stream look dramatic, but NSUserDefaults already maintains an in-process cache and it doesn't hit disk on every read. More importantly, NSUserDefaults shows up as a performance bottleneck in debug builds but is already quite fast in optimized (Deployment) builds. The 27K calls in 3 seconds sounds bad, but the actual CPU cost in a release build is likely negligible. Have you profiled with Instruments Time Profiler on an optimized build to confirm there's a measurable difference?

Specific issues with the implementation

  1. Full cache nuke on every write: setWithoutSideEffectsObject:forKey: calls [sPreferenceCache removeAllObjects] and [sPreferenceExplicitlySetKeys removeAllObjects], then only repopulates the one key being written. This means every other cached value is evicted and valueIsExplicitlySetForKey: can return wrong results for other keys until they're re-read. Just
    updating the single key would be simpler and correct.
  2. Full re-read of all keys on invalidation: PreferencesCurrentObservedValues() reads every preference key from NSUserDefaults to build a comparison snapshot. This runs on every NSUserDefaultsDidChangeNotification, partially defeating the purpose of the cache.
  3. Redundant invalidation paths: The code registers both KVO on every preference key and listens for NSUserDefaultsDidChangeNotification. A single preference change fires both, causing redundant cache clears.
  4. Two separate caching systems: The iTermUserDefaults.m change adds its own independent mini-cache with its own lock for enableAutomaticProfileSwitchingLogging. If the main cache works, this shouldn't need its own mechanism.
  5. Relaxed memory ordering on the generation counter: memory_order_relaxed on ARM (Apple Silicon) means you could theoretically see the generation update without the corresponding cache clear being visible. This should use acquire/release semantics.
  6. KVO on hundreds of keys: Registering KVO for every key in defaultValueMap plus deprecated keys against NSUserDefaults is itself non-trivial overhead.

Unrelated changes

The iTermRuleTest.m API fix, iTermPasteHelperTest.m missing return, and project.pbxproj framework search path additions are unrelated to caching. I'm happy to take those as a separate PR if you'd like — they're useful on their own.

Bottom line

I'm not convinced this addresses a real performance problem in release builds, and the added complexity (locks, generation counters, dual invalidation, two caching layers) introduces new risks for correctness. If you can show a measurable CPU time improvement in an optimized build via Instruments, I'd be happy to revisit. Otherwise I think the complexity/benefit
tradeoff doesn't work out.

@gastonmorixe
Copy link
Copy Markdown
Author

Hi George,

Thank you for iTerm!

I ran into this while debugging unrelated log activity, my main concern was the amount of log spam it was generating. I then started to wonder whether it might also have performance implications, though I was less concerned about that once I realized these reads may not actually be hitting disk.

Please treat this PR as a proof of concept, and feel free to do whatever you think is best with it, including closing it.

If I have time, I'll dig into it further and run some performance benchmarks.

Thanks

@gnachman
Copy link
Copy Markdown
Owner

gnachman commented Apr 9, 2026

Yeah, the log spam is annoying. There are probably more targeted fixes that would make more sense, like caching specific keys that are hit very frequently. If you have some that cause you pain let me know and it's easy to add them to the caching list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants