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

[rush] Avoid cold start overhead by allowing a toolchain process to continue running (as with "webpack --watch") #1151

Open
wbern opened this issue Mar 14, 2019 · 7 comments
Assignees
Labels
blocked We can't proceed because we're waiting for something

Comments

@wbern
Copy link
Contributor

wbern commented Mar 14, 2019

I made a minor note about this functionality in this thread: #1122. This turned out to be really important for us in order to make the switch to Rush, as frontend developers.

What?
Rush currently offers incremental builds, which is great for building as little as possible. However, due to the monolithic nature in the industry of frontend development, there are many tools that ship "watch" instances of themselves, webpack for example, that will keep a running instance of itself, and process changed files immediately as they change. If you are running many small apps (micro-frontends), this might be bearable with the way Rush builds today, but for the larger monolithic webpack-powered apps, this is simply way too much time spent waiting for builds to complete.

What I am requesting is the ability to let build scripts executed by Rush request themselves to be considered "complete", and be sent to background so that Rush can proceed to execute the next script that may or may not have relied on the previous one(s).

Why?
While Webpack 5 is going to specialize in saving the current cache to disk, monolithic applications are literally everywhere, and with the presence of frameworks like Angular, React and Vue, they're likely not going away anytime soon.

At the same time, nodejs offers a complete cross-platform solution for IPC between parent processes and child processes. Surely this should not be a difficult thing to do.

How?
I have made a PR to this Repo that shows how simple it is to do it.

After incorporating this change, all you would need in a typical webpack config is:

plugins: [
    {
        apply(compiler) {
            compiler.hooks.done.tap(
                'MyPlugin',
                (/* compilation */) => {
                    if(compiler.options.watch) {
                        process.send('rush-continue');
                    }
                },
            );
        }
    }
]

After doing this, Rush build will complete and say rush rebuild (46.61 seconds), but will keep running instead of exiting with a zero code. Pressing CTRL + C will exit it, and all IPC-connected spawned processes will exit in the background as well.

This PR of course needs to be improved, as the following issues remain:

  • IPC is enabled by default in the PR. This may introduce breaking changes and generally reduce performance. This should be an opt-in feature. I tried to add it to the options but it was a little difficult for me, so I chose to exclude it from the PR.
  • When the task resolves itself, the writer is closed later on in the code. This means that any stdout/stderr needs to be suppressed in order for the code to work.

Enabling this functionality would make us able to use Rush in production, and finally move from Lerna's open-ended nature and into a documented and scalable monorepo manager which is Rush.

Thanks for reading.

@qm3ster
Copy link

qm3ster commented Mar 14, 2019

In addition to incremental build performance, isn't hot-module-reloading an important usecase of webpack --watch?

@octogonz
Copy link
Collaborator

I have made a PR to this Repo that shows how simple it is to do it.

@wbern I think you might have forgotten to hyperlink the repo that demos your feature. :-P I'm very curious to see that.

@octogonz
Copy link
Collaborator

What I am requesting is the ability to let build scripts executed by Rush request themselves to be considered "complete", and be sent to background so that Rush can proceed to execute the next script that may or may not have relied on the previous one(s).

Is the idea that you want Rush to spawn a separate --watch build process for each project folder? And then when you CTRL+C Rush, it takes care of killing all these processes?

If so, how scalable is this for a repo with 1000+ projects in it?

Also is there a race condition, e.g. if an application starts compiling before a library that it depends on has been built?

@wbern
Copy link
Contributor Author

wbern commented Mar 20, 2019

Is the idea that you want Rush to spawn a separate --watch build process for each project folder? And then when you CTRL+C Rush, it takes care of killing all these processes?
Yes. And I believe that the only way to achieve this in a clean way, is to use the ipc via nodejs. It worked really well when I hacked it into my PR anyway.

Also is there a race condition
My findings have been that, despite having dozens of webpack watch instances, is that they are really good at sorting themselves out. That has never been the issue for us, instead computer resources has been one major issue.

If so, how scalable is this for a repo with 1000+ projects in it?
It really isn't. And that's a really good point. My users today struggle between having freedom of navigating between npm packages and having to compile large things they don't need. Typical monorepo thing, right? As for the performance, bear with me for this post. For now, imagine a 10 project monorepo.

My naive approach was, that rush build could be triggering watch instances, or it wouldn't even know about it (except that ipc is enabled).

The rush build is complete, it would not perform any more logic, but still not exit from terminal, since there are watch scripts running async in the background.

The problems and solutions I guess would be

Problem 1. Rush doesn't know about which processes have gone to the background.
Solution: Easy enough to fix, and the reason for fixing it is below.

Problem 2. Rush build has already ended, nothing more can be done.
Solution: Incorporate the logic of my PR into the rush "watch" functionality. Since rush watch is inherently and arguably designed for this sort of functionality, it can dish out "rush build" instances when it thinks it is necessary, keeping close eye on existing rush builds that it has spawned and making sure that those projects running in watch mode are excluded from the next build. How watch knows about which rush builds holds which watch instances of projects can probably be solved via simple api calls, since it seems like you communicate with rush-lib that way today already (which is great for this use case).

Problem 3. How is this going to scale with 1000+ projects
Solution: It scales terribly if we would just leave it as is with above solution, but since we are now fleshing out brand new functionality which is rush watch, we'd more than likely be able to create

  • options for each project (preferWatch: true) that will help us to only watch the heaviest projects that we hate to rebuild.
  • general option, maxAllowedWatchInstances (note that there may be better terminology than watch, but if we were to start assuming a new script in each package called say build:watch, that would really ease users into this kind of functionality.
  • general option, watchInstanceInactivityTimeout. If no ipc message is sent for a while, rush should close the watch instance altogether.
  • project or general option, buildXTimesBeforeStartingWatchInstance, which perhaps should be an object with both a timer and the amount of times (so if a rebuild happens 3 times under a minute, start a watch instance instead).

I get really excited thinking about this functionality, I hope you see what I mean.

@wbern
Copy link
Contributor Author

wbern commented Mar 26, 2019

As a small update, I have started coding something that works outside of rush.

Basically rush builds projects, and the projects create job requests which connect to an ipc server that determines if there's a job running already or not, and if there is, waits for that job to finish before exiting, so rush can work as normal.

Putting this functionality inside rush would make things easier for other devs, but at the same time, it might break design.

@octogonz
Copy link
Collaborator

octogonz commented Mar 26, 2019

I get really excited thinking about this functionality, I hope you see what I mean.

I agree this is a kickass feature, and we should aim to make it as easy as possible for people to use. If there are toolchain-specific complexities, perhaps Rush could provide two modes: "Basic" where Rush does everything for you using naive assumptions, and "Advanced" where Rush gets things going, but then uses a contract (e.g. IPC) to hand off the real work to a custom service (e.g. that understands idiosyncrasies of a particular tool chain).

I apologize that I haven't had time to give this feature the attention it deserves. My main design feedback is that there should really be a single central service that watches all the files in the repo folder. It should certainly be driven by per-project configurations, as it needs to keep the projects isolated; we shouldn't even assume that the files being "watched" are being consumed by the same task type (webpack, jest, sass whatever). But without a central coordinator, the design seems like it will fall down very quickly as we scale.

For example, I have a small Rush repo with only one library and one application. What I do today is manually run webpack-dev-server in the library folder, and then I run a second webpack-dev-server in the application folder. When I edit a source file for the application, it works fine. When I edit a source file for the library, the library rebuilds correctly, but the application fails to build because it detects the half-written output of the library bundler. After it fails to compile that, it doesn't retry because apparently the notification granularity isn't good enough to signal completion of the second half of the write operation. (I work around this by touching an application source file to force a retry.) There's probably ways to fix this of course. But this scenario shows another obvious problem, that the library folder is being "watched" by two different processes -- that ain't gonna scale.

The tsc --watch option has the right idea of trying to watch ALL files in a monorepo. But it makes a very naive assumption that (1) the TypeScript compiler is the only build task that needs to be incremental, and (2) that all these source files are building using the exact same version of the TypeScript compiler. Both those assumptions are invalidated by even a relatively small real world example like web-build-tools.

If you get some "down time" hanging out at the office watercooler or doing the family dishes or whatever, I'd encourage you to think in depth about the right solution to monorepo watching. If you can work out a solid design, it's very likely people will show up to help you code it as a Rush feature. We all absolutely share your excitement about this idea. :-)

@halfnibble halfnibble self-assigned this Jun 21, 2019
@octogonz octogonz changed the title [rush] allow build scripts to run in the background (eg. webpack --watch support) [rush] Allow custom commands to run in the background (eg. webpack --watch support) Jul 9, 2020
@octogonz octogonz added the blocked We can't proceed because we're waiting for something label Jul 9, 2020
@octogonz octogonz changed the title [rush] Allow custom commands to run in the background (eg. webpack --watch support) [rush] Avoid cold start overhead by allowing a toolchain process to continue running (as with "webpack --watch") Jul 9, 2020
@VanCoding
Copy link

VanCoding commented Jan 18, 2023

Since handling non-terminating watch processes in task-runners is a common problem, maybe we can come up with a protocol, that can be used by all watch-tasks and task-runners.

Here's a proposal of mine: https://github.com/VanCoding/task-graph-protocol

I'd really like to get the discussion started here: VanCoding/task-graph-processor#3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked We can't proceed because we're waiting for something
Projects
Status: Needs triage
Development

No branches or pull requests

5 participants