Skip to content

fix: ignore stale substep timestamps when computing target compilation duration#249

Merged
fortmarek merged 2 commits intoMobileNativeFoundation:masterfrom
fortmarek:fix/stale-substep-timestamps
Apr 24, 2026
Merged

fix: ignore stale substep timestamps when computing target compilation duration#249
fortmarek merged 2 commits intoMobileNativeFoundation:masterfrom
fortmarek:fix/stale-substep-timestamps

Conversation

@fortmarek
Copy link
Copy Markdown
Collaborator

@fortmarek fortmarek commented Apr 24, 2026

Summary

addCompilationTimesToTarget (and addCompilationTimesToApp) currently subtract target.startTimestamp from the latest substep's compilationEndTimestamp. For incremental Xcode builds this can produce a negative compilationDuration because Xcode reuses substep entries from prior build sessions for files that didn't need recompilation. Those substeps keep their original (older) timestamps and are not flagged wasFetchedFromCache, so the existing filter doesn't exclude them, and the chosen "last compile substep" can have ended long before the target started.

This PR filters those stale substeps out by also requiring compilationEndTimestamp >= parent.startTimestamp.

Concrete example

A real .xcactivitylog from an incremental build (404 targets, ~52s wall clock, only one target's sources had changed):

Before After
Targets with negative compilationDuration 364 / 404 0 / 404
Targets with zero 39 403
Targets with positive 1 (0.41s) 1 (0.41s)

The single target with positive duration was the only one with source changes requiring real compilation; the rest correctly report 0. Without the fix, those 403 targets each report values around -1118s, which downstream consumers (e.g. systems storing compilationDuration as an unsigned integer) wrap into nonsensical huge numbers.

Test plan

  • Added testParseTargetCompilationTimesIgnoresStaleSubsteps covering a target whose only substep ended before the target started; expects compilationDuration == 0 and compilationEndTimestamp == startTimestamp.
  • Existing testParseTargetCompilationTimes, testParseAppCompilationTimes, and testParseAppNoopCompilationTimes still pass.
  • Re-parsed an affected real-world .xcactivitylog and confirmed all compilationDuration values are non-negative.

🤖 Generated with Claude Code

@fortmarek fortmarek marked this pull request as ready for review April 24, 2026 12:58
@fortmarek fortmarek requested a review from pepicrft April 24, 2026 12:58
@fortmarek fortmarek force-pushed the fix/stale-substep-timestamps branch from bc8266e to 39f5c7b Compare April 24, 2026 13:10
fortmarek and others added 2 commits April 24, 2026 18:24
…n duration

For incremental builds, Xcode reuses substep entries from previous build
sessions for files that don't need recompilation. Those substeps keep
their original (older) timestamps and are not flagged
`wasFetchedFromCache`. The previous logic in `addCompilationTimesToTarget`
took the latest substep `compilationEndTimestamp` regardless of whether
it actually ran in this build session, which produced a negative
`compilationDuration` whenever the chosen substep predated the target's
`startTimestamp`.

Filter out such stale substeps by requiring
`compilationEndTimestamp >= parent.startTimestamp` in both
`addCompilationTimesToTarget` and `addCompilationTimesToApp`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: fortmarek <marekfort@me.com>
Inline the new filter clause across two lines instead of expanding it
into a four-line block, so the file stays at 400 lines (the SwiftLint
file_length limit).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: fortmarek <marekfort@me.com>
@fortmarek fortmarek force-pushed the fix/stale-substep-timestamps branch from 39f5c7b to bf56c7c Compare April 24, 2026 16:24
@fortmarek fortmarek merged commit a014323 into MobileNativeFoundation:master Apr 24, 2026
4 checks passed
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