I had hesitated because I had thought others had patches that
implemented the /ShowIncludes parsing, and I wanted to see how things
had worked for them before I just landed my own implementation. To be
honest, it's probably my fault that I didn't respond to the mails. I
always fear I'm losing patches that way. :(
On Wed, Feb 15, 2012 at 8:09 AM, Evan Martin <mar...@danga.com> wrote:On Tue, Feb 14, 2012 at 9:56 PM, Scott Graham <sco...@chromium.org> wrote:
I had hesitated because I had thought others had patches that> I'd like to work on getting .d files baked on Windows. I had a brief look at
> the deplist branch which seems like the best candidate for this right now.
> deplist is not currently mergeable with master (looks like it just needs a
> little love in src/graph.cc in LoadDepFile/List).
>
> Feedback was relatively positive I think for including deplist, and it's an
> (optional) superset of functionality while potentially improving
> performance. Are there objections or outstanding issues to resolve? Or is it
> just a matter of testing and doing the work?
implemented the /ShowIncludes parsing, and I wanted to see how things
had worked for them before I just landed my own implementation. To be
honest, it's probably my fault that I didn't respond to the mails. I
always fear I'm losing patches that way. :(
OK, I agree it's probably good to experiment a little. I'll try using the deplist branch for a while, and see how it goes.
Some broad thoughts on this branch:
I agree that it needs experimentation. If we end up concatenating .d
files, which sounds necessary for Windows, we'll need a different file
format. Since the primary point of the new format is to make Windows
work better, it'd be better to figure that out before committing to a
path.
For example another option is to use a Real Database for storing
dependency info (e.g. the equivalent of ninja-deplist-helper could all
talk to some central file). SQLite is the obvious one, and Tony
suggested that leveldb would make sense:
http://code.google.com/p/leveldb/ .
I was hoping someone who has more experience on Windows would be able
to say "We tried X, Y, Z, and concluded that Y is the best way
forward." Perhaps that person will be you. :)
> Specifically | syntax doesn't work because CreateProcess is used to launch
> which doesn't implicate a shell.
>
> I can put cmd /c everywhere for now, but for something that's going to be
> used on every invocation of the compiler, it seems like avoiding that would
> be better.
Playing devil's advocate, is it a real problem? I had figured that,
even if Windows process startup is 50ms, 50ms is still small relative
to the time of a compile. And since this happens as part of the
compile step, it's both not in the critical path and it parallelizes.
> I'd prefer to switch it around and have it look something more
> like:
>
> ninja-deplist-helper -f cl -o a.dep -- cl /nologo /showIncludes /c a.c ...
>
> Does that seem too ugly?
That seems fine to me. I had avoided it because interprocess
communication on Windows is a mystery to me.
Another alternative is that Ninja itself could expose some endpoint
(named pipe on Windows or Unix domain socket on Linux) that the
subprocesses could stream into. Going back to the database idea, this
would allow central management of dependency data.
> I'd also be tempted to have it suppress some of cl's noisy messages too,
> like echoing filename.cc, and "Generating code" and so on, so that we can
> get one line output as on other platforms.
It's not obvious to me how much of this is Ninja's domain and how much
of it is up to the downstream project. In general I like as little
output as possible but maybe others like a lot (I was surprised to
learn that I had to argue to reduce Chrome's Mac build logs should be
under 50mb per build).
In all, I welcome your experimentation and insight. And I think your
gyp-related changes for Chrome will be mostly independent of all of
this anyway so those are not wasted time.
For example another option is to use a Real Database for storing
dependency info (e.g. the equivalent of ninja-deplist-helper could all
talk to some central file). SQLite is the obvious one, and Tony
suggested that leveldb would make sense:
http://code.google.com/p/leveldb/ .
> I can put cmd /c everywhere for now, but for something that's going to bePlaying devil's advocate, is it a real problem? I had figured that,
> used on every invocation of the compiler, it seems like avoiding that would
> be better.
even if Windows process startup is 50ms, 50ms is still small relative
to the time of a compile. And since this happens as part of the
compile step, it's both not in the critical path and it parallelizes.
> Specifically | syntax doesn't work because CreateProcess is used to launchPlaying devil's advocate, is it a real problem? I had figured that,
> which doesn't implicate a shell.
>
> I can put cmd /c everywhere for now, but for something that's going to be
> used on every invocation of the compiler, it seems like avoiding that would
> be better.
even if Windows process startup is 50ms, 50ms is still small relative
to the time of a compile. And since this happens as part of the
compile step, it's both not in the critical path and it parallelizes.
That seems fine to me. I had avoided it because interprocess
> I'd prefer to switch it around and have it look something more
> like:
>
> ninja-deplist-helper -f cl -o a.dep -- cl /nologo /showIncludes /c a.c ...
>
> Does that seem too ugly?
communication on Windows is a mystery to me.
In bash, you could do `cl ... > >(ninja-deplist-helper ...)` to always
get the exit code of cl (ninja-deplist-helper probably doesn't fail?
:-) ). Maybe cmd has something similar?
Nico
On Sat, Mar 3, 2012 at 12:33 PM, Scott Graham <sco...@chromium.org> wrote:
> More on windows deps files and loading times for those interested. All
> tests/times are on Chromium's main DLL.
>
> I wrote a dependency database to hold the equivalent of .d files. ninja and
> deps parser helper use it via shared
> memory: https://github.com/sgraham/ninja/blob/deplist/src/dep_database.cc.
>
> This cut the hot cache deps load time from just over 3000ms to 2400ms (and
> way way better for cold cache). With that change, only about 60ms of 2400ms
> is getting the raw deps off of disk and parsing them into StringPiece
> vectors.
>
> Of the remaining 2.4s, 2.3s is (was) here:
>
> { METRIC_RECORD("depdb add in-edges");
> // Add all its in-edges.
> for (vector<StringPiece>::iterator i = deps.begin();
> i != deps.end(); ++i, ++implicit_dep) {
> *implicit_dep = GetDepNode(state, *i);
> }
> }
>
> I had been building ninja with VS 2008. I just tried with VS2010 instead,
> and that improved that block from 2.3 to 1.6s which is a nice easy
> improvement I should have thought of before.
>
> The deplist branch also didn't have the improved (and maybe fixed?) version
> of adding in-edges that uses StringPiece directly rather than converting to
> string. Merging that improved performance further down to 1.2s.
>
> Removing a few METRIC_RECORDs that I'd put into heavily called functions cut
> another 150ms. (-> ~1s).
the way I read this is that switching from msvc2008 to 2010, rebasing
the deplist branch on top of trunk, and removing a few METRIC_RECORDs
brings warm cache perf from 3s to 1.6s already. Did I get that right?
Thanks,
Nico
Nice work!
One way to avoid stat() is if ninja could ask an external process (an
oracle) whether a file has changed.
For what it's worth, the dependency graph in Ninja has arrows going
"both ways": both from inputs to outputs and from outputs to inputs.
This was because I had thought that originally I would need to use an
oracle process like this.
Currently, when you ask Ninja to build an output, we follow the arrows
back to the inputs, stat'ing everything along the way.
But conceptually (and older code even did this) once you recognized
some input was dirty you could recursively mark every dependent output
as also dirty.
pcc even added logic similar to this to handle the "restat" case,
where we think an output is dirty, but once we run the dependent
command we discover it didn't change the output, which requires us to
do the same "forward" (from input->output) traversal of the dependency
graph marking files as clean.
All of this is a long way of saying that it shouldn't be *too* hard,
if you had a process (or even just subcomponent of ninja) that fed to
ninja "hey, this file has changed" events, for ninja to dynamically
track exactly what needs to be built at all times. (That was in fact
how I had originally envisioned it would work.)
--
Thiago
On Mon, Mar 5, 2012 at 1:22 PM, Thiago Farina <tfa...@chromium.org> wrote:
On Mon, Mar 5, 2012 at 6:40 AM, Philip Craig <phi...@pobox.com> wrote:Doesn't tup do this?
> Would this Oracle be a persistent process (daemon), hooked up to monitor
> file-system changes, using inotify or similar?
>
AFAIK yes. It has a daemon that keep track of the dependency graph in a sqlite DB and these daemon can be notify by several way
Any chance of this getting merged? ninja is sort of useless on windows
with depend information... :)
Thanks.
-Bill