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

App Launch Time Slow:sentrycrashdl_getBinarylmageForHeader #4618

Open
kingnight opened this issue Dec 11, 2024 · 7 comments
Open

App Launch Time Slow:sentrycrashdl_getBinarylmageForHeader #4618

kingnight opened this issue Dec 11, 2024 · 7 comments

Comments

@kingnight
Copy link

Platform

iOS

Environment

Production

Installed

Manually

Version

8.37.0

Xcode Version

15.1

Did it work on previous versions?

No response

Steps to Reproduce

Image

Info from Xcode Organizer:

  • 100 ms spent in this code path during a 1156 ms launch
  • iPhone 13
  • 18.1.1 (22B91)
        SentrySDK.start { options in
            #if DEBUG
            options.enabled = false
            #else
            options.enabled = true
            #endif
            if AppBundle.isReleasePackage() {
                options.dsn = xxx
            }else{
                options.dsn = xxx
            }
            options.enableNetworkTracking = false
            options.enableCaptureFailedRequests = false
            options.sampleRate = 0.5
            options.tracesSampleRate = 0.3
            options.profilesSampleRate = 0.3
            options.appHangTimeoutInterval = 3
            options.beforeSend = { event in
                if let type = event.exceptions?.first?.type,type == "App Hanging" {
                    if let frames = event.exceptions?.first?.stacktrace?.frames,
                       !frames.isEmpty {
                        for item in frames.reversed() {
                            guard let function = item.function else { continue }
                            if AppDelegate.containsSentryWhiteListedStrings(function){
                                return nil
                            }
                        }
                    }
                }
                return event
            }

            SentrySDK.configureScope { scope in
                if let data:Data = ULog.shared.getActivityLog(){
                    scope.addAttachment(Attachment(data: data, filename: "crash.log"))
                    ULog.shared.removeAllActivityLog()
                }
            }
        }

Expected Result

reduce app launch time

Actual Result

23% slow launch time

Are you willing to submit a PR?

No response

@philipphofmann
Copy link
Member

Thanks for reporting this, @kingnight. It would be helpful for us to know how many binary images you roughly have in your app. Could you maybe set a breakpoint here or add a log message and tell us how many images are in the cache?

- (NSArray<SentryBinaryImageInfo *> *)getAllBinaryImages
{
@synchronized(self) {
return _cache.copy;
}
}

Ideas on how to fix this:

  1. Speed up sentrycrashdl_getBinaryImageForHeader.
    @supervacuus, as your native knowledge is way better than mine, do you see room for speed-ups in this method?
    bool
    sentrycrashdl_getBinaryImageForHeader(const void *const header_ptr, const char *const image_name,
    SentryCrashBinaryImage *buffer, bool isCrash)
    {
    const struct mach_header *header = (const struct mach_header *)header_ptr;
    uintptr_t cmdPtr = firstCmdAfterHeader(header);
    if (cmdPtr == 0) {
    return false;
    }
    // Look for the TEXT segment to get the image size.
    // Also look for a UUID command.
    uint64_t imageSize = 0;
    uint64_t imageVmAddr = 0;
    uint64_t version = 0;
    uint8_t *uuid = NULL;
    for (uint32_t iCmd = 0; iCmd < header->ncmds; iCmd++) {
    struct load_command *loadCmd = (struct load_command *)cmdPtr;
    switch (loadCmd->cmd) {
    case LC_SEGMENT: {
    struct segment_command *segCmd = (struct segment_command *)cmdPtr;
    if (strcmp(segCmd->segname, SEG_TEXT) == 0) {
    imageSize = segCmd->vmsize;
    imageVmAddr = segCmd->vmaddr;
    }
    break;
    }
    case LC_SEGMENT_64: {
    struct segment_command_64 *segCmd = (struct segment_command_64 *)cmdPtr;
    if (strcmp(segCmd->segname, SEG_TEXT) == 0) {
    imageSize = segCmd->vmsize;
    imageVmAddr = segCmd->vmaddr;
    }
    break;
    }
    case LC_UUID: {
    struct uuid_command *uuidCmd = (struct uuid_command *)cmdPtr;
    uuid = uuidCmd->uuid;
    break;
    }
    case LC_ID_DYLIB: {
    struct dylib_command *dc = (struct dylib_command *)cmdPtr;
    version = dc->dylib.current_version;
    break;
    }
    }
    cmdPtr += loadCmd->cmdsize;
    }
    buffer->address = (uintptr_t)header;
    buffer->vmAddress = imageVmAddr;
    buffer->size = imageSize;
    buffer->name = image_name;
    buffer->uuid = uuid;
    buffer->cpuType = header->cputype;
    buffer->cpuSubType = header->cpusubtype;
    buffer->majorVersion = version >> 16;
    buffer->minorVersion = (version >> 8) & 0xff;
    buffer->revisionVersion = version & 0xff;
    if (isCrash) {
    getCrashInfo(header, buffer);
    }
    return true;
    }
  2. We could move sentrycrashdl_getBinaryImageForHeader to a bg thread and make calls to SentryBinaryImageCache blocking if sentrycrashdl_getBinaryImageForHeader didn't fill in the information yet. This could fix the problem of slowing down the app launch, but the tracer adds debug meta info from the SentryBinaryImageCache when finishing the app start transaction. So we might have to move that to a bg thread as well.

@philprime philprime moved this from Needs Discussion to Needs More Information in Mobile & Cross Platform SDK Dec 18, 2024
@kingnight
Copy link
Author

@philipphofmann how many binary images : 1072

_cache __NSArrayM * @"1072 elements" 0x00000003005ed260

in debug mode

@supervacuus
Copy link

Speed up sentrycrashdl_getBinaryImageForHeader.
@supervacuus, as your native knowledge is way better than mine, do you see room for speed-ups in this method?

@philipphofmann, I can only see two immediately obvious optimization paths.

  1. The highest cost per segment iteration is the string comparisons of the segment names using strcmp. Using memcmp on the fixed 16-byte name field might be more performant.
  2. Another thing is to terminate the loop early after gathering all required fields and maybe even prevent further segment checks once vmsize and vmaddr are retrieved.

Independent of these, 100ms still sounds like a lot for simple code processing 1000+ modules (~100us/module). I guess isCrash is always false when feeding the cache, right? This could also be an environmental issue.

@philipphofmann
Copy link
Member

Thanks for the update, @kingnight.

Independent of these, 100ms still sounds like a lot for simple code processing 1000+ modules (~100us/module). I guess isCrash is always false when feeding the cache, right? This could also be an environmental issue.

Yes, isCrash is always false when feeding the cache. @supervacuus, do you have any clue why it could be so slow?

@kahest
Copy link
Member

kahest commented Jan 9, 2025

After talking to @supervacuus we think it'd be best if we found a way to reproduce and profile this. @kingnight would you be able to provide a minimum reproducible example?

@kingnight
Copy link
Author

@kahest it's difficult to provide example ,but I set breakpoint in return _cache.copy;

most modules in _cache are system private frameworks , it is right?

Image

another part

Image

should exclude system private frameworks?

@philipphofmann
Copy link
Member

We could create a sample app with loads of dependencies or use an existing open-source app such as Firefox-iOS and try to reproduce the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Status: Needs More Information
Development

No branches or pull requests

5 participants