Use a Confinement Queue to Protect IncrementalCompilationState

[CodaFi]

This class is serendipitously used only from the main queue for the
Driver's usual use cases. But we have no reason to expect clients of
libSwiftDriver to manipulate this class in the same manner. Provide a serial
confinement queue and protect the mutable members of
IncrementalCompilationState with it.

In the future, we ought to drop the queue and use actors.

rdar://75744005

[CodaFi]

@swift-ci test

[akyrtzi]

It would be good to consider at some point using a Lock wrapper over pthread_mutex_t, since in general it can be more efficient than locking via dispatch queue.

[davidungar]

Thank you for undertaking this fix. I would like to see two changes:

1. You wrote a nice comment, but I think it could be better. How about something more like what I suggested?
2. More importantly, unless I'm missing something, the synchronization in getJobs does not seem to be sufficient. Again, see my inline comments. Do you agree?

[davidungar]

Nice comment! May I offer a suggestion? "The foo is a bar" can usually be strengthened with active voice--who is doing what to whom? Also, here I would be tempted to mention the first wave, too. Something like:

"When performing an incremental compilation, the driver must determine which source files much be compiled and which may be skipped. This determination is complicated by the fact that one compilation may reveal the need to perform another compilation if, for instance, the user has changed a declaration in one file that another (unchanged) file, depends upon. These decisions are made here."

Then follow up with the "how":

"Additionally the IncrementalCompilationState must account for the batching of compilations into jobs."

And a chronology:

"It starts with the planning process, which creates the jobs for a non-incremental compilation and passes them of to the InitialCompilationState constructor, which builds the ModuleDependencyGraph and initializes two key fields: mandatoryJobsInOrder, and jobsAfterCompiles. The driver will then schedule the mandatory jobs, which are always needed. This phase is also called the first wave.

Next, as each job finishes, collectJobsDiscoveredToBeNeededAfterFinishing will discover additional jobs that must be run. collectJobsDiscoveredToBeNeededAfterFinishing is also called after each of these jobs. This phase is called the "second wave". When there is no longer any possibility of requiring additional jobs, the driver will run jobsAfterCompiles."

[CodaFi]

I think this would make for better information in a document that describes the implementation details of the incremental build.

[davidungar]

Yes, a document and a pointer is good, but that proposal does not answer my objection:
What bothers me is the general practice if comments that merely summarize the "what" without much, if any, of the "why". The comment, as is, feels like a "what" summary, with very little why.

Why do we have comments in the code at all? To help future maintainers. The most vital help is that which cannot be understood from reading the code, especially non-local information. I don't mean to single this PR out; many comments I read in the whole llvm/swift-compiler community fail this test.

Sure, what I wrote may feel like a bit much, but as it stands--no such document as you propose exists as of this writing--your comment could be more helpful by adding a little more of the nonobvious, nonlocal information.

However, I won't block the PR for this. I will, however, unresolve the conversation.

[CodaFi]

@swift-ci test

[davidungar]

LGTM, thanks!

[CodaFi]

@swift-ci test

[CodaFi]

⛵