Maximum number of namespaces to reload?

mtruyens commented 7 years ago

In my fairly large codebase, some central namespaces are require'd by many other namespaces. If I happen to make an (even trivial) change to such a central namespace, then my browser essentially locks up for several minutes.

To given an example: adding a space to one such central namespace, causes a recompilation process of about 12 seconds. Next, Chrome is locked up for 3 minutes, until finally 97 dependent namespaces are reloaded by Figwheel. Conversely, a hard refresh of the page takes only about 4 - 5 seconds...

In practice, I now try to take care that before any change is made to a central namespace, I temporarily stop Figwheel's autobuilding process, and then re-start it after I made the change and manually reloaded the page. If I forget doing so, then I usually kill the webpage in order to move on, except if the current state of the webpage is really important.

Would it perhaps be a good idea to have a setting that causes Figwheel to issue a visual warning explaining that it will not perform a reload if the number of namespaces to reload is larger than some defined number (e.g., 30)? This would then give the user the opportunity to perform a manual hard refresh of the page, which is much quicker. (Of course, an even better solution could be to give the user an ad-hoc choice of either proceeding with the slow reloading, or stopping it altogether.)

bhauman commented 7 years ago

It shouldn't take 3 minutes to load 97 dependent namespaces.

Its difficult to diagnose a problem like this with out more details.

Two possibilities come to mind:

There is a dependency graph traversal problem that is caused by a loop in the dependency graph that cljc self referencing is causing. (I have a suspicious that something like this exists, but haven't had a concrete case to inspect.) Or its simple a bad dependency graph algorithm, but three minutes seems too long.
Some of the reloaded files have some load time side effects that are causing the process to hang.

Have you tried :reload-dependents false and then fall back to reloading the browser on larger changes?

mtruyens commented 7 years ago

I had already tried with :reload-dependents false (actually that's my default setting), that doesn't help.

However, I think you hit the nail with the dependency graph traversal issue. I indeed have about 10 CLJC-files that are self-referencing in order to have macros in them.

How can I help you get to the bottom of this issue?

bhauman commented 7 years ago

It would be helpful to create a minimal example. Trying to find a case where the graph traversal goes off the rails.

Even maybe identifying the graph nodes that are self referencing because they shouldn't be ...

This may ultimately be a CLJS compiler issue.

The dependency graph is available client side here: https://github.com/bhauman/lein-figwheel/blob/master/support/src/figwheel/client/file_reloading.cljs#L78

The topo sort algorithm is here: https://github.com/bhauman/lein-figwheel/blob/master/support/src/figwheel/client/file_reloading.cljs#L117

With the two main functions calling it here: https://github.com/bhauman/lein-figwheel/blob/master/support/src/figwheel/client/file_reloading.cljs#L137-L144

mtruyens commented 7 years ago

My findings, after digging into this issue:

None of the namespaces that contained self-macros, depended on itself. More general, there were no loops in my dependency graph.
The slowness seems to be simply caused by the high number of namespaces that have to be sorted, in combination with, I think, not the most efficient algorithm for topo-sorting.
I did a quick-and-dirty- comparison with the easy-to-use dependency graph & topo-sorting algorithm used by com.stuartsierra.dependency:
- 1329 total number of keys in the :dependents map, of which 273 belong to my own project (1056 were "immutable" ones, such as the goog.* namespaces)
- com.stuartsierra.dependency takes 569ms to do a full topological sort of all my 273 namespaces;
- Figwheel's current algorithm takes 47 seconds to calculate the dependents for a fairly central (but certainly not the most central) namespace in my project...

bhauman commented 7 years ago

Well this will need to be fixed then.