dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.71k stars 1.06k forks source link

dotnet-build should compile incrementally #4403

Closed TheRealPiotrP closed 4 years ago

TheRealPiotrP commented 8 years ago

dotnet-compile and therefore dotnet-publish is not considering current artifacts when choosing to recompile a project. This is slowing down both operations.

krwq commented 8 years ago

By not considering current artifacts do you mean each of them separately or should publish use dotnet compile's artifacts too?

TheRealPiotrP commented 8 years ago

separate the issues. dotnet-compile should recognize it has already created artifacts for a given project and avoid re-generating them until inputs have changed. dotnet-publish happens to call dotnet-compile and so it's affected by the runtime performance of dotnet-compile.

davidfowl commented 8 years ago

We should change the description of this bug, publish should have nothing to do with it. We should actually describe what we mean by incremental and clearly explain how we want to do it (because it's pretty important to do it properly). The issue is a little vaugue

TheRealPiotrP commented 8 years ago

publish calling compile makes it messy.

Feel free to clean up description, though. So far this issue is just to track the concept and no design is proposed.

cdmihai commented 8 years ago

Incremental compilation depends on the ability to detect when it's not safe to run. Safety discussed on dotnet/cli#382

TheRealPiotrP commented 8 years ago

MS Build-style incremental compile would be a great first step [compare timestamps between inputs and outputs]. You're right that this has gaps which we're trying to address per dotnet/cli#382 but there's a lot of moving parts there and I want to be sure we can start iterating more quickly before that story fleshes out.

cdmihai commented 8 years ago

Here is a proposal of the user experience with incremental compilation.

Incremental compilation requires some safety checks, so we need to check for these and only enable incremental compilation if all safety checks pass (issue dotnet/cli#382).

If a project passes all safety checks, it compiles incrementally by default:

1

Incremental Compilation is turned off when it’s not safe to run. Users receive a message suggesting they could improve their build times if they make their build spec incremental friendly:

2

Users can inspect why their build is not incremental:

3

Users can consult our wiki to read more about the safety checks and how to address them: https://github.com/cdmihai/cli/wiki/Addressing-Incremental-Compilation-Warnings

Incremental compilation gets automatically activated once the user upgrades their build specification to be incremental friendly:

1

Notes:

krwq commented 8 years ago

Hey @cdmihai, the analyze option which you are adding seems to be super useful in general although I have concern if this should be a switch --analyze or rather a separate command like dotnet-compile-analyze.

I personally feel like the code you are trying to add right now adds unnecessary complexity in here while it could probably be a self containing module. In example when I look at dotnet compile --build-profile I'm not quite sure what this is gonna do but if you renamed this to dotnet build analyze --build-profile it all starts making sense and I probably wouldn't have to type help to figure out what will this do.

In addition to that we are currently stabilizing commands like dotnet, dotnet compile, dotnet publish, dotnet new and any of its dependencies. Any changes and additions just bring risk of introducing new bugs.

If we make it a separate command it would make it easier for you to iterate over your code and would make more sense from user experience and code perspective.

For tips like "Your build can be optimized" sound super nice to me as I'm working on a multiple project solution. I think quite common scenario is to work on a solution with 1 or 2 projects where this message wouldn't be very welcome and I'd rather use this "bad" postcompile steps than write a separate script. Even if it takes 0.5ms to produce the message (and information how fast this is not quite available or obvious) I still wouldn't like to see any of these messages and I would try to disable them which would not be clear how to do.

cdmihai commented 8 years ago

@krwq

I see your concerns, however there's a few things that are unclear to me. Here are a few cases (some do require modifications of the dotnet-compile code)

Invariants to all cases:

Cases:

  1. dotnet-build becomes the new entry command to compilation. Users invoke build and not compile. Build does safety checking and chooses either incremental or non-incremental compilation (as maybe different strategies for the same template hook). dotnet-compile code is retired and merged as a phase into build (as a bonus we might also eliminate the need to invoke a new compile process per each project dependency). Alternative: dotnet-compile does not get retired and remains as is: incremental compilation on the other hand gets fused into build (so it cannot be invoked in an unsafe manner) and then users can call compile if they want to bypass safety checking and incremental compilation
  2. dotnet-compile remains the entry command for compilation. It needs to be modified to call dotnet build in order to reason whether it's safe to do incremental or non incremental (disadvantage: extra process invocation might negate speed gains). It then needs to be modified to support different strategies for compilation.
  3. We don't add an extra build command, nor an extra --build-profile flag. We modify compile to log safety checking, but it logs all the safety messages to a separate log file. Then, instead of instructing a user to add an extra flag to see the safety messages, we just tell them to look into the incremental-safety-checks.log file.
    • this requires similar modifications to compile as option 2.

I prefer case 1. with the alternative, since it is open to new functionality but it keeps dotnet-compile closed to modifications (except maybe pulling out some common code).

What are some other opinions?

dannyvv commented 8 years ago

If we can't do this work in dotnet-compile.exe then my vote would also be for the proposal of adding a new dotnet-build.exe.

Since the check and the incrementality are closely related and should go hand-in-hand, it will imply the logic for incrementality will also be outside of dotnet-compile into dotnet-build.exe.

Given those constrains dotnet-build would be the most reasonable.

cdmihai commented 8 years ago

Implemented in dotnet/cli#761

pranavkm commented 8 years ago

What build of dotnet-cli has this fix? I'm using 1.0.0.000973 and I don't see a build-profile flag.

cdmihai commented 8 years ago

@pranavkm This is in master and rel/1.0.0. The flag is --build-profile and it's on the build verb

pranavkm commented 8 years ago

Perhaps I'm missing something obvious, but I ran dotnet build twice in a row and expected it to not recompile the second time around. Here's what I'm seeing:

> dotnet build --build-profile

Project Microsoft.AspNetCore.Mvc.Abstractions will be compiled because expected outputs are missing.
Compiling Microsoft.AspNetCore.Mvc.Abstractions for .NETFramework,Version=v4.5.1
Compilation succeeded.
    0 Warning(s)
    0 Error(s)
Time elapsed 00:00:02.9326431

> dotnet build

Project Microsoft.AspNetCore.Mvc.Abstractions will be compiled because expected outputs are missing.
Compiling Microsoft.AspNetCore.Mvc.Abstractions for .NETFramework,Version=v4.5.1
Compilation succeeded.
    0 Warning(s)
    0 Error(s)
Time elapsed 00:00:02.9440321

This is using v1.0.1.001096. What am I doing wrong?

cdmihai commented 8 years ago

@pranavkm

the --build-profile flag only searches for incremental preconditions that might make incremental unsafe. If such conditions are present, incrementality is turned off.

In your case, there are no preconditions, so nothing happens. I opened dotnet/cli#1050 to improve on this message.

Regarding your situation, can you please run the above with verbosity logging (dotnet --verbose build)? That should print out which output items are missing.

pranavkm commented 8 years ago

The result of dotnet --verbose build: https://gist.github.com/pranavkm/e3df70b00d8677947767

I get the same output running it multiple times over.

cdmihai commented 8 years ago

That is really weird, no outpus are being printed. Though here is the code that prints them if they're there: https://github.com/dotnet/cli/blob/master/src/Microsoft.DotNet.Tools.Builder/CompileContext.cs#L157

Are you sure you have the latest code and that dotnet.exe binds to the correct path?

Can you please do:

dotnet new
dotnet restore
dotnet --verbose build

It should show the missing items (dll, pdb, etc).

fearthecowboy commented 8 years ago

Perhaps I'm missing something obvious, but I ran dotnet build twice in a row and expected it to not recompile the second time around. Here's what I'm seeing...

I see this exact same behavior on projects that have resources.

It appears that dotnet-build will rebuild any project that has resources, regardless of anything changing whatsoever.

cdmihai commented 8 years ago

I opened an issue to track this: dotnet/cli#1136

@pranavkm Does your project have resources as well?

mvinca commented 7 years ago

How do you do the equivalent of "dotnet build --build-profile" on today's offering?