-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
[Packager] Ever since GA, Packager has been *really slow* for the first time. #341
Comments
What is GA? |
General Availability. :) JG 301.785.6030 :: @moduscreate :: sent from my mobile device ::
|
One thing that has happened is I moved the packager's node_modules to the root. Making them crawlable by the packager which is probably slowing down things. This was to fix an urgent issue #225 |
Actually just tried this on a couple of machines and it seems to be running ok, it's not taking more than 5 seconds. Can you try restarting your computer? |
That's my point. 5 seconds on a cold start for packager is painfully slow. It wasn't this way before :( JG :: sent from my mobile device ::
|
Yes, we should fix it. |
Agreed we can and should make it faster, but I'm curious: how often and for On Fri, Mar 27, 2015 at 10:54 AM, Amjad Masad notifications@github.com
|
Not often :). I just happened to notice it after a kernel panic (debugging core audio implementation). This also surfaced another strange thing: while the first cold-boot request is made to the packager, hit cmd+r. That induces a second request, causing your app to render twice. This is most noticeable in "SampleApp" JG 301.785.6030 :: @moduscreate :: sent from my mobile device ::
|
I think the render twice issue has been fixed - does it still repro for you on master? -Spencer
|
@jaygarcia can you open another issue for the render twice? |
The packager has been really slow after setting a couple of root directories even when it's hot. When I make a small change to one file and load the bundle in my browser, it takes about 6-7 seconds. My hypothesis is that it's scanning deep into node_modules to see if there are any Haste directives (@amasad - would you know if this is the case?). I have some big devDependencies (gulp, coffeescript, etc.) and when I |
You could try adding the dev dependency paths to the blacklist (react-native/packager/blacklist.js - sharedBlacklist). In fact, the packager could check the root package.json for devDependencies, assuming these are just tools. |
That, actually, probably won't help. The better fix would be to add a more elaborate glob pattern in the react-packager/src/service/index.js that lists the individual dirs ignoring the node_modules devDependencies. |
Is there ever a need to scan node_modules for @providesModule? I thought about it a bit and the only case I could think of was for react-native to be able to pull in some helper modules from react-tools. Either node_modules/react-tools could be whitelisted, or those files in question could just ship with react-native proper (they may already, I haven't checked recently). |
Just for react-tools, as far as I know. We're trying to do work to get away from @providesModule for that too. |
@ide Good point, although Internally, I think we have some modules living in node_modules with @providesModules. But we can make this a setting. We still need to read the directories to build the dependency graph but should be much faster than reading the files to look for providesModules. |
@ide btw, did you update recently? The bluebird promise patch should get you 30% speed improvement. |
@amasad I'm using up to 08b48ca which is fairly recent. Some setting to limit searching under |
I was mistaken that we didn't need to read those files, since the way the packager works now is that it builds up the graph ahead of time we still need to read the files to parse out the requires. Saving the docblock parsing isn't that big of a win. On a 14 second graph build time it saves 2 seconds. I'm going to try using watchman for the directory reading and file stating. I believe that'll have a bigger impact. |
Additionally, caching the graph is not out of the question here but I always try to keep it as the last resort. |
It might end up involving a bunch of work but I imagine lazily building the graph would help a lot here and not involve caching (good call). Ex: since my root file (ExampleApp.js) doesn't transitively require |
The node_modules part can be lazily constructed, however, haste modules cannot because the modules are not tied to the filesystem location. I think having the graph be half lazy / half eager would be more complex than caching. We have a cache module for the transformer and it's pretty stable, I'll see if it can be easily reused for this. Btw, I just reread your earlier comment. I had missed that this was slow for you even after start up time.
Ooh, this shouldn't be the case at all. No recrawl happens when you change a file it should just get reprocess the module that changed and rebuild the package with everything else from cache. Can you double check the numbers? Maybe post the logs from your terminal |
It's fast for me now actually. I'm not sure what changed, other than today's commits (only c32d9bc#diff-e5979e1e4c8039af168a3114bcbac5ffL67 looks related...). |
@ide good. I still want to make sure startup time is better but, I think, it's not as important as the change/reload cycle being sub-second. |
@jaygarcia how's the packager working for you now? |
@jsierles DUDE! such a huge difference!! React packager ready. [10:46:03 AM] request:/jsx/index.ios.bundle Thanks :) |
Next sync a speed up will land which will make node_modules dependency reading lazy, i.e. unless you require a node module, that file will not be read. |
I only noticed this since GA. Am i alone in this?
The text was updated successfully, but these errors were encountered: