bhauman / lein-figwheel

Figwheel builds your ClojureScript code and hot loads it into the browser as you are coding!
Eclipse Public License 1.0
2.88k stars 208 forks source link

reloads slow since 0.3.8 #264

Open runningskull opened 9 years ago

runningskull commented 9 years ago

Hi Bruce. First off, I love Fighweel - thanks for it!

There’s still parts of what’s going on that I don’t quite understand, so dumping what I know that seems relevant. I’m happy to answer any further questions.

I have :recompile-dependents false to avoid a CLJS compiler bug & :reload-dependents false figwheel option based on a few similar figwheel Issues.

I’m working on a moderately complex project (few dozen files) & when I change a file that’s required in a lot of places, Figwheel >= 0.3.8 seems to get stuck in a tight loop in the build-topo-sort function for a long time (> 1 minute) and freezes the browser tab). Initial & incremental cljs compilation happens quickly - just the reloading is affected.

I added some print’s, and I’m seeing the inner topo-sort function (topo-sort*) getting called many times with the exact same deps, and seeing “cycles” of the same deps over and over. for instance, I’ll see (each line is one pass through topo-sort*)

deps: figwheel.connect
deps: my-ui.pages.core
deps: my-ui.core

repeat itself many times throughout the loop.

That all may not be enough to go by. I’m happy to provide a full dump of what’s printed (it’ll be big), or any other information/logs that would be helpful.

EDIT: just saw that when I change the one file (x.cljs), figwheel_server.log reports notifying browser that file changed: ... for each of the compiled js files that require's x.cljs. I expected to see only x.js, so seemed worth mentioning.

bhauman commented 9 years ago

There are a bunch of strange things here. You shouldn't get notifying browser that file changed for all the dependencies. This makes me think that you may be outputing your compiled assets on a watched-path or using cljx or something like that.

Another strange thing is "my-ui.pages.core" should be "my_ui.pages.core" the separator should be an underscore.

So I'm thinking there are some paths crossing big time.

Can you create a small example project of these behaviors with figwheel 4.1? Just namespaces will work.

One more thing there is a significant optimization that I should do which is if :reload-dependents is false and only one file comes up the pike I should just reload that file full stop.

bhauman commented 9 years ago

Another possibility is that you are missmatched. An old figwheel backend with a new figwheel front end. This is a recipe for disaster.

runningskull commented 9 years ago

Thanks - I'll check into those & try to put together a minimal case if I can't spot anything I'm doing that's wonky.

As for the hyphen/underscore - that was me fat-fingering instead of copy/pasting like I should have - it's indeed an underscore. :)

bhauman commented 9 years ago

did you ever learn anything more here?

runningskull commented 9 years ago

Sorry for the delay - I tabled it for the time being so I could move forward w/ the app itself (older version of figwheel works for now, as does boot), but I still intend to put together a self-contained reproducible case. Feel free to close this issue if you want, and I'll find it again once I've got more info.

lorddoig commented 8 years ago

Had a very similar issue recently - turns out we had a :figwheel-always on a heavily required namespace.

alexandergunnarson commented 8 years ago

I have approximately the same issue as others have been experiencing. I have about 60 namespaces in my project (possibly more — it's a large library I'm creating), and even when I modify one that affects only about 6-8 other namespaces, I have approximately the same issue as runningskull. That is, Figwheel (I'm using 0.5.0-2) "seems to get stuck in a tight loop in the build-topo-sort function for a long time (> 1 minute) and freezes" both the browser tab and the developer tools portion (I'm using Chrome). Again, as runningskull said, "Initial & incremental cljs compilation happens quickly - just the reloading is affected." After about 2 seconds, I see that the process main (from Figwheel) ramps up to 100-120% CPU — it's compiling — and then once it's finished, the Chrome processes for the browser tab and the dev tools ramp up to 40%, then 50%, then eventually 90% CPU. Like runningskull, I added some printlns, and topo-sort* ends up being called an astonishing number of times:

screen shot 2016-03-10 at 10 57 06 am screen shot 2016-03-10 at 10 57 16 am screen shot 2016-03-10 at 10 57 22 am

Then it continues in that vein (repeating) deeper and deeper (I got to about 30 before the CPU was at around 95% and the console wasn't printing anything out anymore), never actually fully reloading the code.

This problem doesn't happen if I reload a semi-top-level namespace (i.e. has ~0-5 namespaces that depend on it). Maybe this is an n-factorial type of explosion? I am almost certain there are not circular dependencies because I've had those, and this kind of behavior is not characteristic of circular dependencies as far as I can tell.

Also, this problem didn't happen when I had a bunch of warnings in commonly-used dependency-namespaces. When I fixed them all (and thereby enabled reloading of them), then this problem began.

Nothing is marked as ^:figwheel-always. In fact, I (inconveniently) marked a few namespaces as :figwheel-no-load to try to alleviate the problem but no luck.

Any ideas?

bhauman commented 8 years ago

Well it sounds like a circular dependency.

But I really need a minimal case in order to ferret this out.

alexandergunnarson commented 8 years ago

So I tried something out. I compiled the CLJS using lein clean && lein figwheel, postwalked the produced .js files, and created a directed graph out of the goog.provide and goog.require statements at the beginning of each file, using the loom library. Then I did a topo-sort (also using loom) and there were no cycles found. The topo-sort using loom was also pretty instantaneous.

Perhaps this test case is insufficient to determine cyclic dependencies, but it seems unlikely.

I'm still trying other things and may produce a minimal test case at some point, though since I can't pinpoint the problem to a particular namespace yet, I don't even know what would constitute a minimal test case yet.

UPDATE: I also cleaned out the compiled js files and ran lein cljsbuild once successfully. Admittedly it took 40 seconds without advanced compilation because of how much code I have, but still. There's no evidence of circular dependencies there.

alexandergunnarson commented 8 years ago

After some further fanciness with read-string with {:cond-read :allow :features #{:cljs :cljc}} on the ns forms of my source code, plus using Johnson's algorithm to find all cycles, I found one problematic circular dependency which hadn't shown up in any other way. Figwheel didn't tell me (I mean, it did say "ANALYSIS ERROR: EOF while reading in file predicates.cljc on file null, line null, column null"... so that was super helpful), cljsbuild didn't tell me (it ran fine, as I had said), and parsing the compiled .js didn't tell me. Good thing I have these tools now to find circular dependencies — it took way too much time to figure this out!

So I removed the circular dependency and everything works fine now. Phew.

What I had done (and to be honest I still have this in some files, but Figwheel was hanging on only that one particular file) was define a macro, e.g. my-macro in repo/code.cljc, and in the ns form, I did a conditional compilation like #?(:cljs (:require-macros [repo.code :refer [my-macro]])). So CLJS was referring to a CLJ macro in the same file, which is apparently acceptable. CLJ was not referring to itself and technically CLJS was not referring to itself. Thus the subtleness of the bug. What I still don't get is it works for the other namespaces, just not the one I had to fix... oh well.

bhauman commented 8 years ago

I think it could be super helpful to have a small example case. I may be able figure out if this a bug in figwheel or the tooling somewhere.

I can almost create an example from your description but if you could post the explicit code that's causing the blow up it would be greatly appreciated

runningskull commented 8 years ago

@alexandergunnarson - thanks for putting in the legwork to dig into that. Have you learned anything else about why the one particular namespace was troublesome?

Also if you get the urge to open source those loop detection scripts... ;)

bhauman commented 7 years ago

https://github.com/bhauman/lein-figwheel/commit/6abba55b486881b6bab4fd2d23799e9c8beaab8e

This commit may have helped this issue.

alexandergunnarson commented 7 years ago

I will check that out! Thanks for letting me know about it!

danielcompton commented 6 years ago

@runningskull @alexandergunnarson is this still an issue for you? If not, this issue can probably be closed?