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

Decide the defaults L&F of the multithread terminal view #210

Closed
ppalaga opened this issue Nov 12, 2020 · 8 comments
Closed

Decide the defaults L&F of the multithread terminal view #210

ppalaga opened this issue Nov 12, 2020 · 8 comments
Labels
wontfix This will not be worked on

Comments

@ppalaga
Copy link
Contributor

ppalaga commented Nov 12, 2020

  1. The view should immediately claim its preferred number of lines (possibly adding empty lines at the end) so that the top status line does not move up during the build as more active threads are added.

  2. Maybe the "preferred number of lines" default should be all available terminal lines. If we do this, rollingWindowSize can be computed dynamically and it does not need to be a preference. I am not sure whether this a good idea. I'd have to try an see.

  3. The current policy of showing the full rollingWindowSize where possible starting from the beginning often looks strange on my 24 core machine: 8 threads have log lines and the rest has none. Maybe it should be all or none. WDYT?

  4. The upper bounds of per-thread number of log lines should not take the number of active threads into account, so that the status lines of the individual threads stay fixed on the same line during the build.

  5. Maybe in situations, when the build happens to be linear, because the dependency graph does not allow any concurrency, noBuffering could be activated automatically. (This is trivially true for single module builds) moved to Maven-like rolling output when the build happens to be linear #269

@gnodet
Copy link
Contributor

gnodet commented Nov 12, 2020

It should be possible to enhance the smart builder to compute the maximum concurrency of a project, so that we could also use it as a maximum for the number of threads to display.

@gnodet
Copy link
Contributor

gnodet commented Nov 16, 2020

It should be possible to enhance the smart builder to compute the maximum concurrency of a project, so that we could also use it as a maximum for the number of threads to display.

I've pushed a commit on my repo (https://github.com/gnodet/mvnd/tree/dag-width) which should be able to bring an upper-bound to the maximum concurrency level. We should use it in order to know the max number of lines to use.
I also agree, that if the max concurrency is 1, we should default to the noBuffering option (not sure if we need this behaviour to be configurable).

Adding a full-screen mode shouldn't be very difficult to do. Whether it should be the default or not, I'm not sure. If the project is known to have a small maximum concurrency (see above) and if the rolling window is not enabled, I don't think we should reserve the whole screen. Though we could add an option and see what's best.

@gnodet
Copy link
Contributor

gnodet commented Nov 28, 2020

@ppalaga I'm willing to reconsider the default behaviour of mvnd so that it gets closer to maven.
A simple switch should allow to switch to a parallelized build, but I think it makes sense to be less disruptive as projects are not always ready for a parallel build (especially relying on the order of the modules can cause problems). Building projects which aren't really ready for parallel builds make things a bit complicated with the current mvnd.
That would also ease the adoption of mvnd a lot I think.
As an alternative, we could have a different executable /script such as mvdp(p for parallel).

So that would mean:

  • making sure output buffering is disabled if threads == 1
  • default to threads=1
  • default to use the standard builder instead of the smart one
  • add a switch to easily enable the --threads and --builder options

@ppalaga
Copy link
Contributor Author

ppalaga commented Nov 28, 2020

+1 for the simple switch that would configure all those things at once. But I'd personally find it a pity to give up on making pressure on the whole ecosystem to move towards parallelization. I mean if we stop offering it as a default, both end users and plugin developers will give up on improving things. Detecting the problems related to parallel builds, naming them, complaining about them will become less common and the progress will slow down. I think we can afford to stay a bit disruptive ;)

@gnodet
Copy link
Contributor

gnodet commented Dec 10, 2020

The simple switch has been implemented in #248 .

@gnodet
Copy link
Contributor

gnodet commented Feb 9, 2021

@ppalaga what about closing this one ? I'm quite pleased the way it is now.

@ppalaga
Copy link
Contributor Author

ppalaga commented Feb 10, 2021

Point 1. does not seem to be implemented yet and I'd be OK with moving it into a separate issue.

I do not mind abandoning the rest.

@gnodet
Copy link
Contributor

gnodet commented May 19, 2021

Raised #414 to reserve lines. We can always improve later.

@gnodet gnodet closed this as completed May 19, 2021
@ppalaga ppalaga added this to the No fix/wont't fix milestone Oct 26, 2021
@gnodet gnodet added the wontfix This will not be worked on label Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants