perf: reduce session load/reload time

[romangolev]

Summary

A set of independent fixes to cut down pyRevit session load/reload time, found by
instrumenting a real dev session's runtime.log and following the evidence.

• pyrevitlib/pyrevit/loader/sessionmgr.py / pyrevit/coreutils/envvars.py: userconfig's
  verify/upgrade/save cycle was re-running from scratch on every "clean" IronPython engine
  spin-up (once per extension's startup script, since each gets a fresh engine). Gated behind a
  new CONFIGUPGRADED AppDomain env var so it only runs once per Revit process. Also caches the
  reflection lookup for the C# LoadSession method instead of re-scanning the AppDomain on every
  reload.
• pyrevitlib/pyrevit/versionmgr/updater.py: check_for_updates() (called by the "Update"
  smart button's __selfinit__ on every session load/reload) did a full git-remote-fetch
  network round trip each time. Now cached for 15 minutes via AppDomain env vars.
• pyrevitlib/pyrevit/loader/sessionmgr.py: auto-update now runs after the ribbon UI is built
  (perform_postload) instead of before (perform_preload), so a slow or offline update check
  can't delay the ribbon's first paint.
• dev/pyRevitLoader/pyRevitAssemblyBuilder/UIManager/SessionManagerService.cs: extension
  assembly build+load (PASS 1) now runs in parallel instead of a sequential loop; stale-assembly
  and appdata cleanup are queued on a background task instead of blocking the load.
• dev/pyRevitLoader/pyRevitAssemblyBuilder/AssemblyMaker/AssemblyBuilderService.cs: added
  locks around the two pieces of shared mutable state the now-parallel build pass touches.
• dev/pyRevitLoader/pyRevitExtensionParser/IniFile.cs: PyRevitConfig's underlying INI file
  was fully re-read and re-scanned line-by-line on every single property read (hit repeatedly
  per extension during UI build). Now parsed once into an in-memory dictionary, invalidated on
  write or on the existing session-reload cache-clear hook.
• dev/pyRevitLoader/pyRevitAssemblyBuilder/UIManager/IconsHandling/{BitmapCache,IconManager}.cs:
  the decoded-icon bitmap cache was thrown away and rebuilt on every LoadSession() call (first
  load and every reload), so every ribbon icon got redecoded from disk on every reload. Now a
  single process-lifetime cache; entries self-invalidate via file last-write-time so an icon a
  developer edits and reloads still picks up the change.

Test plan

• dotnet build on pyRevitAssemblyBuilder.csproj and pyRevitExtensionParser.csproj -
  clean, no new warnings
• Full pyRevitExtensionParserTest suite - same pre-existing baseline failure count before
  and after (verified via git stash on the changed files), no new failures
• py_compile on all changed Python files
• Verified in a live Revit 2025 session via runtime.log: userconfig upgrade/save now runs
  once per process instead of once per extension; check_for_updates network fetch count
  dropped as expected; no new errors/warnings in the log

[devloai]

PR Summary:

This PR reduces pyRevit session load/reload time through several independent optimizations:

• Gates userconfig upgrade/save to once-per-process via an AppDomain env var (was once per extension engine)
• Caches check_for_updates() network fetch results for 15 minutes via AppDomain env vars
• Moves auto-update from preload to postload so update checks don't delay ribbon first paint
• Parallelizes extension assembly build+load (PASS 1) and queues cleanup on background tasks
• Adds locks around shared mutable state in the now-parallel build path
• Caches IniFile parsed content in-memory (was re-reading the file on every property read)
• Makes the icon BitmapCache a process-lifetime static with last-write-time invalidation (was discarded on every reload)

Review Summary:

Reviewed all 10 changed files across Python and C#. The performance approach is sound — each optimization targets a real bottleneck identified via runtime.log instrumentation, and the thread-safety additions for the parallelized build pass (locks on _loadedPyRevitAssemblyNames and the env dict) are correctly scoped. The IniFile in-memory cache and the shared BitmapCache are well-designed with appropriate invalidation strategies.

Five comments recorded, two medium and three low severity:

1. Medium: check_for_updates() caches False (including no-internet) for 15 minutes — a transient network blip will suppress update detection for the full TTL window. Consider only caching positive results.
2. Medium: has_pending_updates() returns None on fetch failure, which gets cached as falsy for 15 minutes — same suppression risk as #1 but from a git-fetch failure rather than no-internet.
3. Low: IniFile._valueCache/_sectionCache lack thread synchronization — latent risk now that the build pass is parallel, though currently no config reads happen on the parallel path.
4. Low: Moving auto-update to postload creates a re-entrant LoadSession call when updates are applied — the outer session's cleanup/logging runs against a replaced session.
5. Low: Shared BitmapCache makes per-extension ResetAndGetStats() cache hit/miss [PERF] log lines misleading (diagnostic-only, no functional impact).

IPY2712 compatibility verified: AppDomain env var storage, time.time() floats, and True/None boolean semantics all work correctly across clean IronPython engines. Black formatting and PEP8 naming conventions are respected in the Python changes.

Suggestions

• Add a unit test for check_for_updates() caching behavior covering no-internet, fetch-failure, and up-to-date scenarios. Apply
• Run ruff check and black --check on the changed Python files to confirm no formatting violations. Apply

[devloai]

The cache now stores False (no updates / no internet) for 15 minutes. If a user experiences a brief network blip during session load, _check_for_updates_uncached() returns False, which gets cached. Even if connectivity returns moments later, the Update smart button won't detect available updates for up to 15 minutes.

Consider only caching the True (updates available) result, or skip caching when the result was due to no-internet, so a transient outage doesn't suppress update detection for the full TTL window.

result = _check_for_updates_uncached()
# Only cache positive results; a False could be a transient network failure
if result:
    envvars.set_pyrevit_env_var(envvars.CHECKUPDATES_TIMESTAMP_ENVVAR, now)
    envvars.set_pyrevit_env_var(envvars.CHECKUPDATES_RESULT_ENVVAR, result)
return result

actions

Feedback: Rate this comment to help me improve future code reviews:

• 👍 Good - Helpful and accurate
• 👎 Poor - Wrong, unclear, or unhelpful
• Skip if you don't have any strong opinions either way.

[devloai]

has_pending_updates() returns True or None (it has no explicit return False). When has_pending_updates returns None, the if has_pending_updates(repo): check falls through to the else branch which logs "up-to-date" — which is misleading since None means "fetch failed" or "no data", not necessarily "up to date".

This is a pre-existing issue, but now that the result is cached for 15 minutes, a None (falsified to False by the cache layer) from a failed fetch will be cached and suppress update detection for 15 minutes. Consider adding an explicit return False at the end of has_pending_updates and distinguishing "fetch failed" from "up to date" in the logging.

actions

Feedback: Rate this comment to help me improve future code reviews:

• 👍 Good - Helpful and accurate
• 👎 Poor - Wrong, unclear, or unhelpful
• Skip if you don't have any strong opinions either way.

[devloai]

The IniFile._valueCache/_sectionCache fields are read and written without any synchronization. EnsureCacheLoaded() does a non-atomic check-then-fill: if _valueCache is null, it builds the dictionaries and assigns them.

PyRevitConfig.Load() uses double-checked locking to protect the _defaultInstance singleton, but once the instance is created, subsequent concurrent reads through the same PyRevitConfig→IniFile instance hit EnsureCacheLoaded() without a lock. If the C# build path ever accesses PyRevitConfig from the now-parallel BuildAndLoadAllAssemblies loop (e.g., via extension parsing or ParseExtensionByName), two threads could race on the null-check and both try to populate the cache, or one could read a half-constructed dictionary.

Currently BuildExtensionAssembly works with already-parsed ParsedExtension objects and doesn't touch PyRevitConfig directly, so this is latent rather than active. But since the build pass is now parallel, consider adding a lock around EnsureCacheLoaded() (or making the cache fields volatile + using Lazy<>), to future-proof against any config read that may be added to the build path.

actions

Feedback: Rate this comment to help me improve future code reviews:

• 👍 Good - Helpful and accurate
• 👎 Poor - Wrong, unclear, or unhelpful
• Skip if you don't have any strong opinions either way.

[devloai]

Moving auto-update from perform_preload to perform_postload is a good UX improvement (ribbon appears before the update check runs), but note a subtle re-entrancy consequence: updater.update_pyrevit() can call sessionmgr.reload_pyrevit() → load_session() → _invoke_csharp_loadsession() → LoadSession(), which re-enters the C# LoadSession() method while the current LoadSession() is still on the call stack (between ExecuteEntryScript(POSTLOAD_SCRIPT) and the final totalStopwatch.Stop()).

In the original preload placement, the re-entrant LoadSession happened before any C# UI state was built, so there was nothing to corrupt. Now, the outer LoadSession has already built the ribbon and is in its postload phase when the inner reload tears it all down and rebuilds. If the inner LoadSession succeeds, the outer one's remaining lines (_cleanup_output(), totalStopwatch.Stop(), final log) run against a session that was just replaced, producing a misleading "Session loaded" log for the old session.

This only triggers when updates are actually found and applied (not every load), so it's low-frequency. But consider guarding against the re-entrancy, e.g., by having update_pyrevit set a flag that perform_postload checks to skip the remaining cleanup/logging, or by documenting that reload_pyrevit from within postload is expected to short-circuit the rest of postload.

actions

Feedback: Rate this comment to help me improve future code reviews:

• 👍 Good - Helpful and accurate
• 👎 Poor - Wrong, unclear, or unhelpful
• Skip if you don't have any strong opinions either way.

[devloai]

ResetAndGetStats() on BitmapCache resets the global _hits/_misses counters via Interlocked.Exchange. With this shared cache, EmitBuildUIPerfLines (called after each extension's BuildUI) resets counters that are now shared across all extensions and all reloads.

This means the per-extension [PERF] cache hit/miss attribution is now incorrect: the first extension's ResetAndGetStats will pick up hits accumulated during previous sessions (since the cache is process-lifetime), and subsequent extensions will only see hits since the previous extension's reset, not since their own BuildUI started.

The _decodeMs field on IconManager is still per-instance, so decode time attribution is still correct, but the cache hit/miss numbers in the [PERF] log lines will be misleading. Since this is diagnostic-only (no functional impact), it's low priority, but worth noting so the [PERF] lines don't mislead future profiling work.

actions

Feedback: Rate this comment to help me improve future code reviews:

• 👍 Good - Helpful and accurate
• 👎 Poor - Wrong, unclear, or unhelpful
• Skip if you don't have any strong opinions either way.