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

NSBundle bundleWithIdentifier causes slowdown when creating FlutterEngine #37826

Closed
xster opened this issue Aug 8, 2019 · 11 comments · Fixed by flutter/engine#39975
Closed
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) customer: chalk (g3) engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list platform-ios iOS applications specifically

Comments

@xster
Copy link
Member

xster commented Aug 8, 2019

The addition of traversing the bundles by searching for an identifier added in https://github.com/flutter/engine/pull/5986/files#diff-8e54aca7e9d4fd69a57fb86d6ec3dee2R36 adds a 100ms latency when instantiating a FlutterEngine (7ms -> 106ms on an iPod).

It's hard to figure out the original feature request from the PR. Can we revise the approach?

@xster xster added platform-ios iOS applications specifically engine flutter/engine repository. See also e: labels. c: performance Relates to speed or footprint issues (see "perf:" labels) labels Aug 8, 2019
@jamesderlin
Copy link
Contributor

Internally we wanted to support having multiple bundles and not rely on the main bundle when looking up Flutter assets. (I'm still trying to refresh my memory about exactly why... I think we considered it a burden for add2app?)

The +[NSBundle bundleForClass]: call to initialize engineBundle could be moved to where it's used. I'm not sure what to do about the +[NSBundle bundleWithIdentifier:] call. Possibly we could replace it with +[NSBundle bundleWithPath:] (although since the point of finding the bundle is to extract paths from it, probably not). Maybe another possibility would be to change the ordering so that the expensive lookup is done only after checking cheaper sources.

Anyway, I'm happy to ponder this and assist however I can, but I currently am no longer set up to make and test changes, so this probably would be better owned by someone else.

@jamesderlin jamesderlin removed their assignment Aug 12, 2019
@xster
Copy link
Member Author

xster commented Nov 18, 2019

For context, the original feature request was b/112004455

@gaaclarke
Copy link
Member

gaaclarke commented Nov 18, 2019

Screen Shot 2019-11-18 at 1 15 52 PM

FWIW [FlutterDartProject init] takes 4ms on iPhone SE. Debug build at dd77b73

Edit: This was recorded on a full Flutter app, not Add-to-app.

@xster
Copy link
Member Author

xster commented Nov 18, 2019

Could have been fixed already. Feel free to close this.

Previous observations with https://github.com/flutter/engine/pull/5986/files
Screen Shot 2019-11-18 at 1 54 37 PM

and without
Screen Shot 2019-11-18 at 1 54 44 PM

@gaaclarke
Copy link
Member

I measured with the same previous settings, but an Add-to-app scenario and didn't notice any problem either.

Screen Shot 2019-11-18 at 2 20 30 PM

The slowdown is plausible. I just don't know what conditions lead to [NSBundle bundleWithIdentifier:] being so slow. Any ideas?

@chinmaygarde
Copy link
Member

The [discussion on [NSBundle bundleWithIdentifier:]](https://developer.apple.com/documentation/foundation/nsbundle/1411929-bundlewithidentifier?language=objc) has a section on performance implications of this call and suggests alternatives. Specifically:

As an optimization, you can use the bundleWithPath: or bundleWithURL: method instead to avoid file system traversal.

@chinmaygarde chinmaygarde added easy fix P2 Important issues not at the top of the work list labels Jun 15, 2020
@cailianqing
Copy link

cailianqing commented May 11, 2022

image
hi,the problem caused watchdog crash.Is there a repair plan?

@chuckern
Copy link

@gaaclarke Our project use engineGroup, and there are some watchdog crash when create engine.
Could u please kindly check this?
btw: Flutter version is 2.8.1.
image

@jiahaog
Copy link
Member

jiahaog commented Feb 23, 2023

It seems like this could be proportionate to the size of the app. For example, customer: chalk is more impacted by it than the raw hello flutter app. Googlers - see http://shortn/_WcifhO48X6 for details.

@jiahaog
Copy link
Member

jiahaog commented Mar 1, 2023

Tested with build_engine=1 internally - as a quick experiment, removing the call to +[NSBundle bundleWithIdentifier] doesn't break the app, and improves startup of customer: chalk by 30 - 50%. It also seems like the runtime of this method scales to the size of the app bundle.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) customer: chalk (g3) engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list platform-ios iOS applications specifically
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants