Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PLAT-8410] Add device.time to OOM and Thermal Kill events #1355

Merged
merged 2 commits into from May 6, 2022

Conversation

nickdowell
Copy link
Contributor

Goal

Provide a timestamp for OOM and Thermal Kill events so that the dashboard renders breadcrumbs with sensible relative timestamps.

Changeset

Adds a timestamp field to BSGRunContext which is updated from a timer and in response to certain events (so that device.time >= breadcrumbs.last.timestamp)

Sets device.time for OOM and TK events based on the last run's context.

Uses CFAbsoluteTimeGetCurrent() as the source of timing information, which provides a wall-clock time equivalent to [NSDate date].timeIntervalSinceReferenceDate.

Performance

CFAbsoluteTime() is very fast on recent OS versions (~70 nanoseconds on iPhone 5s with iOS 12) where it does not need to call into the kernel.

On iPad 3 running iOS 9 it takes ~1400 nanoseconds (1.4 µs) which while much slower is still reasonable for the frequency at which this code calls it. For comparison, a single character write() to stdout takes ~8 µs.

Considered using mach_approximate_time() which is much faster (~45 nanoseconds on iPad 3) but since it would be difficult (perhaps impossible?) to convert to the required wall-clock time and only old devices / OSes would benefit, the extra complexity does not feel justified.

Testing

Amends OOM barebone E2E scenario to verify presence of device.time.

Manually verified outcome on dashboard using example app.

@github-actions
Copy link

github-actions bot commented May 5, 2022

1 Warning
⚠️ This PR modifies BSGRunContext.h but does not change BSGRUNCONTEXT_VERSION

Bugsnag.framework binary size increased by 648 bytes from 845,360 to 846,008

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.2%    +452  +0.2%    +452    __TEXT,__text
  +0.2%    +272  +0.2%    +272    Symbol Table
  +0.1%    +264  +0.1%    +264    String Table
  +1.5%     +64  +1.5%     +64    Lazy Binding Info
  +1.2%     +32  +1.2%     +32    Binding Info
  +0.9%     +32  +0.9%     +32    __DATA,__const
  [ = ]       0  +0.2%     +24    __DATA,__bss
  +1.1%     +24  +1.1%     +24    __TEXT,__stub_helper
  +1.1%     +24  +1.1%     +24    __TEXT,__stubs
  +1.3%     +20  +1.3%     +20    Indirect Symbol Table
  +1.1%     +16  +1.1%     +16    __DATA,__la_symbol_ptr
  +2.4%      +8  +2.4%      +8    __DATA,__got
  -0.2%      -8  -0.2%      -8    __TEXT,__unwind_info
  -1.2%     -56  -0.7%     -80    [__DATA]
  -1.9%    -492  -1.9%    -492    [__TEXT]
  [DEL]      -4  -9.9%    -652    [__LINKEDIT]
  +0.1%    +648  [ = ]       0    TOTAL

Generated by 🚫 Danger

@nickdowell
Copy link
Contributor Author

nickdowell commented May 5, 2022

⚠️ This PR modifies BSGRunContext.h but does not change BSGRUNCONTEXT_VERSION

Have not changed BSGRUNCONTEXT_VERSION because we have not yet made a release that includes BSGRunContext

@nickdowell nickdowell merged commit 7e85865 into next May 6, 2022
@nickdowell nickdowell deleted the nickdowell/oom-timestamps branch May 6, 2022 09:23
@nickdowell nickdowell mentioned this pull request May 11, 2022
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.

None yet

2 participants