new build log format: hashes of command lines

[Evan Martin]

Ninja stores a log of commands used to build the current tree:
http://martine.github.com/ninja/manual.html#_the_ninja_log

I've merged a change to the format. We now log only a hash of the
command used to generate an output rather than the full command line.
This reduced the build log size for Chrome on Mac from 197mb to 1.6mb,
which makes it much faster to load. From the pull request it improved
Windows load time from 500ms -> 20ms.

We lose the ability to extract the actual commands run from the build
log, which I had used in the past when playing around with
visualization. I don't think it's too useful though, and Nico has a
patch that can mostly reconstruct it. (I wonder if "ninja -t
commands" would suffice for most applications of that sort of thing
anyway.)

[Dirk Pranke]

Ah! I've always wondered what the point of the log was (or, at least,
I've wondered why there was a "compacting log" message) ... it seems
like if people actually want to keep a human-readable build-log
around, that should be a different option and/or a different file to
not slow down the main startup.

-- Dirk

[Evan Martin]

On Fri, Jun 15, 2012 at 2:18 PM, Dirk Pranke <dpr...@chromium.org> wrote:
> Ah! I've always wondered what the point of the log was (or, at least,
> I've wondered why there was a "compacting log" message) ... it seems
> like if people actually want to keep a human-readable build-log
> around, that should be a different option and/or a different  file to
> not slow down the main startup.

For what it's worth, "compacting log" lives on. We append to the log
file as the build progresses, which means a given output file can get
redundant entries for each successive build. "Compacting" is the step
where we read the log and throw out all but the most recent entry per
file.

This area remains one where I'm not as pleased with the design of
Ninja. We definitely need to track *some* per-output state, but it
could have been formatted in any of a number of ways.

[Claus Klein]

Hi Martin,
I have merged in the new source code version.It works fine and is
about 10 times faster on my macbook pro (Darwin) as before:./
build_log_perftest58ms60ms60ms60ms60msmin 58ms  max 60ms  avg 59.8ms

But the code is not as clean as it should:
diff --git a/src/canon_perftest.cc b/src/canon_perftest.ccindex
aaec935..4f23324 100644--- a/src/canon_perftest.cc+++ b/src/
canon_perftest.cc@@ -1,3 +1,4 @@+#include <string.h> // for strlen,
strcopy #include "util.h"  const char kPath[]
=--------------------------------------------------------------------------------------
and...[20/21] CXX build/ninja.oIn file included from /opt/local/
include/gcc47/c++/ext/hash_map:61:0,                 from src/
hash_map.h:74,                 from src/build_log.h:23,               
 from src/ninja.cc:39:/opt/local/include/gcc47/c++//backward/
backward_warning.h:33:2: warning: #warning This file includes at least
one deprecated or antiquated header which may be removed without
further notice at a future date. Please use a non-deprecated interface
with equivalent functionality instead. For a listing of replacement
headers and interfaces, consult the file backward_warning.h. To
disable this warning use -Wno-deprecated. [-Wcpp][21/21] LINK ninja
IMO the ninja build should not use -Wno-deprecated
diff --git a/configure.py b/configure.pyindex 3099ea8..66a192a
100755--- a/configure.py+++ b/configure.py@@ -118,7 +118,7 @@ if
platform == 'windows':         ldflags += ['/LTCG', '/OPT:REF', '/
OPT:ICF'] else:     cflags = ['-g', '-Wall', '-Wextra',-            
 '-Wno-deprecated',+              '-Wdeprecated',               '-Wno-
unused-parameter',               '-fno-rtti',               '-fno-
exceptions',

Please can you fix it?
//RegardsClaus

[Evan Martin]

On Sat, Jun 16, 2012 at 5:53 AM, Claus Klein <claus...@arcormail.de> wrote:
> [20/21] CXX build/ninja.o
> In file included from /opt/local/include/gcc47/c++/ext/hash_map:61:0,
>                from src/hash_map.h:74,
> [...]

> backward_warning.h. To disable this warning use -Wno-deprecated. [-Wcpp]

> IMO the ninja build should not use -Wno-deprecated

As you observe, if you leave -Wno-deprecated in this warning doesn't appear.
If you want to change the compile flags then you're welcome to also
fix the issues that come up by changing them. :)

In this particular case, I believe they deprecated hash_map in
preference for unordered_map but unordered map isn't completely
available in the compilers we'd like to build on. (One page I saw
claimed you need to build with --std=c++0x to use it, which I know
leads to other problems with Clang due to the system headers not
supporting the same version of C++0x as gcc.) I could be completely
wrong, though. If you think I am wrong, I would be happy to look at
your patch that fixes this.

[Konstantin Tokarev]

16.06.2012, 21:11, "Evan Martin" <mar...@danga.com>:

> On Sat, Jun 16, 2012 at 5:53 AM, Claus Klein <claus...@arcormail.de> wrote:
>

>> О©╫[20/21] CXX build/ninja.o
>> О©╫In file included from /opt/local/include/gcc47/c++/ext/hash_map:61:0,
>> О©╫О©╫ О©╫ О©╫ О©╫ О©╫ О©╫ О©╫ О©╫from src/hash_map.h:74,
>> О©╫[...]
>> О©╫backward_warning.h. To disable this warning use -Wno-deprecated. [-Wcpp]
>> О©╫IMO the ninja build should not use -Wno-deprecated

>
> As you observe, if you leave -Wno-deprecated in this warning doesn't appear.
> If you want to change the compile flags then you're welcome to also

> fix the issues that come up by changing them. О©╫:)

>
> In this particular case, I believe they deprecated hash_map in
> preference for unordered_map but unordered map isn't completely
> available in the compilers we'd like to build on.

It's located in tr1 namespace

--
Regards,
Konstantin

[Claus Klein]

Done,
see https://github.com/martine/ninja/pull/335

On Jun 16, 7:11 pm, Evan Martin <mart...@danga.com> wrote:

[Peter Kümmel]

On 15.06.2012 23:03, Evan Martin wrote:
> Ninja stores a log of commands used to build the current tree:
> http://martine.github.com/ninja/manual.html#_the_ninja_log
>
> I've merged a change to the format. We now log only a hash of the
> command used to generate an output rather than the full command line.
> This reduced the build log size for Chrome on Mac from 197mb to 1.6mb,
> which makes it much faster to load. From the pull request it improved
> Windows load time from 500ms -> 20ms.
>

There is a cmake test which now constantly fails after this merge

http://open.cdash.org/testDetails.php?test=150408597&build=2368905

I've checked it, it is definitely the commit

"Only store command hashes in the build log."
https://github.com/martine/ninja/commit/5be83d0b2e4909f39998c98dde9a1393d8e5f1c2

I assume the timing has changed because of the hash calculation,
the test always fails with

Recompacting log...
ninja: ERROR: opening build log: Permission denied


Couldn't be
1. open log file
2. read content
3. close log file
4. calculate hash values

or is it necessary to lock the file until
all hashes are calculated?

Peter

>

[Claus Klein]

It seems a windows specific problem!

My tests on Darwin works fine:
http://open.cdash.org/testDetails.php?test=150436493&build=2369648

And the open/close handling was not changed:
It is open for append while building because the every successful cmd
is written while build.

It is only faster now.

On Jun 17, 1:08 pm, Peter Kümmel <syntheti...@gmx.net> wrote:
> On 15.06.2012 23:03, Evan Martin wrote:
>
> > Ninja stores a log of commands used to build the current tree:
> >    http://martine.github.com/ninja/manual.html#_the_ninja_log
>
> > I've merged a change to the format.  We now log only a hash of the
> > command used to generate an output rather than the full command line.
> > This reduced the build log size for Chrome on Mac from 197mb to 1.6mb,
> > which makes it much faster to load.  From the pull request it improved
> > Windows load time from 500ms ->  20ms.
>
> There is a cmake test which now constantly fails after this merge
>
> http://open.cdash.org/testDetails.php?test=150408597&build=2368905
>
> I've checked it, it is definitely the commit
>

> "Only store command hashes in the build log."https://github.com/martine/ninja/commit/5be83d0b2e4909f39998c98dde9a1...

[Nico Weber]

Hi Peter,

One side effect of that change is that existing build logs are forced to be compacted. Can you check if a change that only changes kCurrentVersion in build_log.cc to something else also triggers this failure (with my change reverted locally)?

Nico

 

Peter

[Claus Klein]

Hi Martin,

I have merged in the new source code version.

It works fine and is about 10 times faster on my macbook pro (Darwin) as before:

./build_log_perftest

58ms

60ms

60ms

60ms

60ms

min 58ms  max 60ms  avg 59.8ms

But the code is not as clean as it should:

diff --git a/src/canon_perftest.cc b/src/canon_perftest.cc

index aaec935..4f23324 100644

--- a/src/canon_perftest.cc

+++ b/src/canon_perftest.cc

@@ -1,3 +1,4 @@

+#include <string.h> // for strlen, strcopy

 #include "util.h"

 

 const char kPath[] =

--------------------------------------------------------------------------------------

and

...

[20/21] CXX build/ninja.o

In file included from /opt/local/include/gcc47/c++/ext/hash_map:61:0,

                 from src/hash_map.h:74,

                 from src/build_log.h:23,

                 from src/ninja.cc:39:

/opt/local/include/gcc47/c++//backward/backward_warning.h:33:2: warning: #warning This file includes at least one deprecated or antiquated header which may be removed without further notice at a future date. Please use a non-deprecated interface with equivalent functionality instead. For a listing of replacement headers and interfaces, consult the file backward_warning.h. To disable this warning use -Wno-deprecated. [-Wcpp]

[21/21] LINK ninja

IMO the ninja build should not use -Wno-deprecated

diff --git a/configure.py b/configure.py

index 3099ea8..66a192a 100755

--- a/configure.py

+++ b/configure.py

@@ -118,7 +118,7 @@ if platform == 'windows':

         ldflags += ['/LTCG', '/OPT:REF', '/OPT:ICF']

 else:

     cflags = ['-g', '-Wall', '-Wextra',

-              '-Wno-deprecated',

+              '-Wdeprecated',

               '-Wno-unused-parameter',

               '-fno-rtti',

               '-fno-exceptions',

Please can you fix it?

//Regards

Claus

[Claus Klein]

Hi again,

the goole web interface in not so good as it should.

I have merged in the new source code version.
It works fine and is about 10 times faster on my macbook pro (Darwin)
as before:
./build_log_perftest
58ms
60ms
60ms
60ms
60ms
min 58ms max 60ms avg 59.8ms


But the code is not as clean as it should:

...
[20/21] CXX build/ninja.o
In file included from /opt/local/include/gcc47/c++/ext/hash_map:61:0,
from src/hash_map.h:74,
from src/build_log.h:23,
from src/ninja.cc:39:
/opt/local/include/gcc47/c++//backward/backward_warning.h:33:2:
warning: #warning This file includes at least one deprecated or
antiquated header which may be removed without further notice at a
future date. Please use a non-deprecated interface with equivalent
functionality instead. For a listing of replacement headers and
interfaces, consult the file backward_warning.h. To disable this
warning use -Wno-deprecated. [-Wcpp]
[21/21] LINK ninja

IMO the ninja build should not use -Wno-deprecated

[Claus Klein]

Hi Martin,

I have merged in the new source code version.

It works fine and is about 10 times faster on my macbook pro (Darwin) as before:

./build_log_perftest

58ms

60ms

60ms

60ms

60ms

min 58ms  max 60ms  avg 59.8ms

But the code is not as clean as it should:

diff --git a/src/canon_perftest.cc b/src/canon_perftest.cc

index aaec935..4f23324 100644

--- a/src/canon_perftest.cc

+++ b/src/canon_perftest.cc

@@ -1,3 +1,4 @@

+#include <string.h> // for strlen, strcopy

 #include "util.h"

 

 const char kPath[] =

--------------------------------------------------------------------------------------

and

...

[20/21] CXX build/ninja.o

In file included from /opt/local/include/gcc47/c++/ext/hash_map:61:0,

                 from src/hash_map.h:74,

                 from src/build_log.h:23,

                 from src/ninja.cc:39:

/opt/local/include/gcc47/c++//backward/backward_warning.h:33:2: warning: #warning This file includes at least one deprecated or antiquated header which may be removed without further notice at a future date. Please use a non-deprecated interface with equivalent functionality instead. For a listing of replacement headers and interfaces, consult the file backward_warning.h. To disable this warning use -Wno-deprecated. [-Wcpp]

[21/21] LINK ninja

IMO the ninja build should not use -Wno-deprecated

diff --git a/configure.py b/configure.py

index 3099ea8..66a192a 100755

--- a/configure.py

+++ b/configure.py

@@ -118,7 +118,7 @@ if platform == 'windows':

         ldflags += ['/LTCG', '/OPT:REF', '/OPT:ICF']

 else:

     cflags = ['-g', '-Wall', '-Wextra',

-              '-Wno-deprecated',

+              '-Wdeprecated',

               '-Wno-unused-parameter',

               '-fno-rtti',

               '-fno-exceptions',

Please can you fix it?

//Regards

Claus

On 15.06.2012, at 23:03, Evan Martin wrote:

[Claus Klein]

Hi again,

the goole web interface in not so good as it should and my posts from
my mail client get lost?

I have merged in the new source code version.
It works fine and is about 10 times faster on my macbook pro (Darwin)
as before:
./build_log_perftest
58ms
60ms
60ms
60ms
60ms
min 58ms max 60ms avg 59.8ms


But the code is not as clean as it should:

...
[20/21] CXX build/ninja.o
In file included from /opt/local/include/gcc47/c++/ext/hash_map:61:0,
from src/hash_map.h:74,
from src/build_log.h:23,
from src/ninja.cc:39:
/opt/local/include/gcc47/c++//backward/backward_warning.h:33:2:
warning: #warning This file includes at least one deprecated or
antiquated header which may be removed without further notice at a
future date. Please use a non-deprecated interface with equivalent
functionality instead. For a listing of replacement headers and
interfaces, consult the file backward_warning.h. To disable this
warning use -Wno-deprecated. [-Wcpp]
[21/21] LINK ninja

IMO the ninja build should not use -Wno-deprecated

[Claus Klein]

Hi Martin,

I have merged in this new version. It works, and is 10 times faster on my macbook (Darwin).

But the code is not clean:

diff --git a/src/canon_perftest.cc b/src/canon_perftest.cc

index aaec935..4f23324 100644

--- a/src/canon_perftest.cc

+++ b/src/canon_perftest.cc

@@ -1,3 +1,4 @@

+#include <string.h> // for strlen, strcopy

 #include "util.h"

 

 const char kPath[] =

--------------------------------------------------------------------------------------

...

[20/21] CXX build/ninja.o

In file included from /opt/local/include/gcc47/c++/ext/hash_map:61:0,

                 from src/hash_map.h:74,

                 from src/build_log.h:23,

                 from src/ninja.cc:39:

/opt/local/include/gcc47/c++//backward/backward_warning.h:33:2: warning: #warning This file includes at least one deprecated or antiquated header which may be removed without further notice at a future date. Please use a non-deprecated interface with equivalent functionality instead. For a listing of replacement headers and interfaces, consult the file backward_warning.h. To disable this warning use -Wno-deprecated. [-Wcpp]

[21/21] LINK ninja

I think, ninja should not ignore such warnings!

Please fix it.

//Regards

Claus

On 15.06.2012, at 23:03, Evan Martin wrote:

[Nico Weber]

I tried reproducing this on Windows. It seems that cmake trunk (on
branch master) doesn't include ninja generator output. The
"recompacting log" step that happens when building ninja itself on
Windows seems to work fine both before and after my patch. Does the
bot heal if you manually remove all .ninja_log files you can find?
And, as asked above, does this problem happen if you just change the
log version number?

>
> Nico
>
>>
>>
>> Peter
>>
>>>
>

[Peter Kümmel]

>
> I tried reproducing this on Windows. It seems that cmake trunk (on
> branch master) doesn't include ninja generator output. The
> "recompacting log" step that happens when building ninja itself on
> Windows seems to work fine both before and after my patch. Does the
> bot heal if you manually remove all .ninja_log files you can find?
> And, as asked above, does this problem happen if you just change the
> log version number?
>

The error happens when there is no log file.
Ninja tried anyway to recompact the file, which gave
the access error.

Here a fix: https://github.com/martine/ninja/pull/338

The cmake branch with ninja enabled is 'next' in
http://cmake.org/gitweb?p=stage/cmake.git

Peter