Update dynamic annotations and move them to base/third_party

Update dynamic annotations and move them to base/third_party

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=47252

[Timur Iskhodzhanov, 2010-05-11 14:13:17]

Hi Alexander, Gregory, Lei,

I'd like to update dynamic annotations files to a more recent version and move
them
to base/third_party to have the same BSD-styled license as we have in
ThreadSanitizer trunk
http://code.google.com/p/data-race-test/source/browse/#svn/trunk/dynamic_anno...
(It doesn't require us to reproduce the copyright notice in the binaries)

To do this,
a) I've created a new build target called dynamic_annotations
   (otherwise we have multiple symbol definition conflicts in base/allocator)
and also
b) I had to manually list
base/third_party/dynamic_annotations/dynamic_annotations.{h,c}
   in base_nacl_win64 source files list
   (otherwise the 32-bit dynamic_annotations target isn't linked into
base_nacl_win64
   and we get "undefined symbol" errors)

Can you please take a look at the CL?
Any suggestions on how to do it better are welcome

Thank you,
Timur Iskhodzhanov

[Alexander Potapenko, 2010-05-11 14:24:01]

I believe your solution is acceptable and splitting dynamic_annotations into two
targets for 32-bit and 64-bit builds will be an overkill.

LGTM

[gregoryd, 2010-05-11 17:31:48]

http://codereview.chromium.org/1992005/diff/15001/16002
File base/base.gypi (right):

http://codereview.chromium.org/1992005/diff/15001/16002#newcode612
base/base.gypi:612: 'third_party/dynamic_annotations/dynamic_annotations.c',
I think creating a 64-bit target for dynamic_annotations will make this
configuration easier to follow. We are planning to merge the generic targets
with win64 targets at some stage, and having a dynamic_annotations64 target will
make it easier to do the right thing.

http://codereview.chromium.org/1992005/diff/15001/16004
File base/dynamic_annotations.h (right):

http://codereview.chromium.org/1992005/diff/15001/16004#newcode11
base/dynamic_annotations.h:11: #include
"base/third_party/dynamic_annotations/dynamic_annotations.h"
This makes following the dependencies difficult. Can we just use the full path
wherever this files is included?

http://codereview.chromium.org/1992005/diff/15001/16005
File base/third_party/dynamic_annotations/dynamic_annotations.c (right):

http://codereview.chromium.org/1992005/diff/15001/16005#newcode41
base/third_party/dynamic_annotations/dynamic_annotations.c:41: #include
"dynamic_annotations.h"
Please provide full path

http://codereview.chromium.org/1992005/diff/15001/16006
File base/third_party/dynamic_annotations/dynamic_annotations.h (right):

http://codereview.chromium.org/1992005/diff/15001/16006#newcode53
base/third_party/dynamic_annotations/dynamic_annotations.h:53: #ifndef
__DYNAMIC_ANNOTATIONS_H__
The include guard should be
BASE_THIRD_PARTY_DYNAMIC_ANNOTATIONS_DYNAMIC_ANNOTATIONS_H

http://codereview.chromium.org/1992005/diff/15001/16006#newcode115
base/third_party/dynamic_annotations/dynamic_annotations.h:115: /* Report that
we are about to signal_all on the condition variable at "cv". */
long line

http://codereview.chromium.org/1992005/diff/15001/16006#newcode189
base/third_party/dynamic_annotations/dynamic_annotations.h:189: /* Report that
we've just got an element from a FIFO queue at address "pcq". */
long line

[Timur Iskhodzhanov, 2010-05-11 19:14:56]

http://codereview.chromium.org/1992005/diff/15001/16002
File base/base.gypi (right):

http://codereview.chromium.org/1992005/diff/15001/16002#newcode612
base/base.gypi:612: 'third_party/dynamic_annotations/dynamic_annotations.c',
Okay, I did this and added a TODO for you.
On 2010/05/11 17:31:48, gregoryd wrote:
> I think creating a 64-bit target for dynamic_annotations will make this
> configuration easier to follow. We are planning to merge the generic targets
> with win64 targets at some stage, and having a dynamic_annotations64 target
will
> make it easier to do the right thing.

http://codereview.chromium.org/1992005/diff/15001/16004
File base/dynamic_annotations.h (right):

http://codereview.chromium.org/1992005/diff/15001/16004#newcode11
base/dynamic_annotations.h:11: #include
"base/third_party/dynamic_annotations/dynamic_annotations.h"
We already have
#include "base/dynamic_annotations.h"
lines in quite a few files in Chromium and in many other projects like
perftools, NaCL, etc.
The long line is much harder to remember.

That's why I'd rather stay with the short line.
On 2010/05/11 17:31:48, gregoryd wrote:
> This makes following the dependencies difficult. Can we just use the full path
> wherever this files is included?

http://codereview.chromium.org/1992005/diff/15001/16005
File base/third_party/dynamic_annotations/dynamic_annotations.c (right):

http://codereview.chromium.org/1992005/diff/15001/16005#newcode41
base/third_party/dynamic_annotations/dynamic_annotations.c:41: #include
"dynamic_annotations.h"
On 2010/05/11 17:31:48, gregoryd wrote:
> Please provide full path

Done.

http://codereview.chromium.org/1992005/diff/15001/16006
File base/third_party/dynamic_annotations/dynamic_annotations.h (right):

http://codereview.chromium.org/1992005/diff/15001/16006#newcode53
base/third_party/dynamic_annotations/dynamic_annotations.h:53: #ifndef
__DYNAMIC_ANNOTATIONS_H__
This guard is copied from the upstream version of dynamic_annotations.h
and I'm consistent with how the guard is done in valgrind.h:
http://src.chromium.org/viewvc/chrome/trunk/src/base/third_party/valgrind/val...
On 2010/05/11 17:31:48, gregoryd wrote:
> The include guard should be
> BASE_THIRD_PARTY_DYNAMIC_ANNOTATIONS_DYNAMIC_ANNOTATIONS_H

http://codereview.chromium.org/1992005/diff/15001/16006#newcode115
base/third_party/dynamic_annotations/dynamic_annotations.h:115: /* Report that
we are about to signal_all on the condition variable at "cv". */
On 2010/05/11 17:31:48, gregoryd wrote:
> long line

Done.

http://codereview.chromium.org/1992005/diff/15001/16006#newcode189
base/third_party/dynamic_annotations/dynamic_annotations.h:189: /* Report that
we've just got an element from a FIFO queue at address "pcq". */
On 2010/05/11 17:31:48, gregoryd wrote:
> long line

Done.

[Timur Iskhodzhanov, 2010-05-11 19:17:03]

Oops, looks like I did a typo in the .gyp file; will notify you when it's fixed
On 2010/05/11 19:14:56, Timur Iskhodzhanov wrote:
> http://codereview.chromium.org/1992005/diff/15001/16002
> File base/base.gypi (right):
> 
> http://codereview.chromium.org/1992005/diff/15001/16002#newcode612
> base/base.gypi:612: 'third_party/dynamic_annotations/dynamic_annotations.c',
> Okay, I did this and added a TODO for you.
> On 2010/05/11 17:31:48, gregoryd wrote:
> > I think creating a 64-bit target for dynamic_annotations will make this
> > configuration easier to follow. We are planning to merge the generic targets
> > with win64 targets at some stage, and having a dynamic_annotations64 target
> will
> > make it easier to do the right thing.
> 
> http://codereview.chromium.org/1992005/diff/15001/16004
> File base/dynamic_annotations.h (right):
> 
> http://codereview.chromium.org/1992005/diff/15001/16004#newcode11
> base/dynamic_annotations.h:11: #include
> "base/third_party/dynamic_annotations/dynamic_annotations.h"
> We already have
> #include "base/dynamic_annotations.h"
> lines in quite a few files in Chromium and in many other projects like
> perftools, NaCL, etc.
> The long line is much harder to remember.
> 
> That's why I'd rather stay with the short line.
> On 2010/05/11 17:31:48, gregoryd wrote:
> > This makes following the dependencies difficult. Can we just use the full
path
> > wherever this files is included?
> 
> http://codereview.chromium.org/1992005/diff/15001/16005
> File base/third_party/dynamic_annotations/dynamic_annotations.c (right):
> 
> http://codereview.chromium.org/1992005/diff/15001/16005#newcode41
> base/third_party/dynamic_annotations/dynamic_annotations.c:41: #include
> "dynamic_annotations.h"
> On 2010/05/11 17:31:48, gregoryd wrote:
> > Please provide full path
> 
> Done.
> 
> http://codereview.chromium.org/1992005/diff/15001/16006
> File base/third_party/dynamic_annotations/dynamic_annotations.h (right):
> 
> http://codereview.chromium.org/1992005/diff/15001/16006#newcode53
> base/third_party/dynamic_annotations/dynamic_annotations.h:53: #ifndef
> __DYNAMIC_ANNOTATIONS_H__
> This guard is copied from the upstream version of dynamic_annotations.h
> and I'm consistent with how the guard is done in valgrind.h:
>
http://src.chromium.org/viewvc/chrome/trunk/src/base/third_party/valgrind/val...
> On 2010/05/11 17:31:48, gregoryd wrote:
> > The include guard should be
> > BASE_THIRD_PARTY_DYNAMIC_ANNOTATIONS_DYNAMIC_ANNOTATIONS_H
> 
> http://codereview.chromium.org/1992005/diff/15001/16006#newcode115
> base/third_party/dynamic_annotations/dynamic_annotations.h:115: /* Report that
> we are about to signal_all on the condition variable at "cv". */
> On 2010/05/11 17:31:48, gregoryd wrote:
> > long line
> 
> Done.
> 
> http://codereview.chromium.org/1992005/diff/15001/16006#newcode189
> base/third_party/dynamic_annotations/dynamic_annotations.h:189: /* Report that
> we've just got an element from a FIFO queue at address "pcq". */
> On 2010/05/11 17:31:48, gregoryd wrote:
> > long line
> 
> Done.

[Timur Iskhodzhanov, 2010-05-11 19:51:03]

Fixed the typo
On 2010/05/11 19:17:03, Timur Iskhodzhanov wrote:
> Oops, looks like I did a typo in the .gyp file; will notify you when it's
fixed
> On 2010/05/11 19:14:56, Timur Iskhodzhanov wrote:
> > http://codereview.chromium.org/1992005/diff/15001/16002
> > File base/base.gypi (right):
> > 
> > http://codereview.chromium.org/1992005/diff/15001/16002#newcode612
> > base/base.gypi:612: 'third_party/dynamic_annotations/dynamic_annotations.c',
> > Okay, I did this and added a TODO for you.
> > On 2010/05/11 17:31:48, gregoryd wrote:
> > > I think creating a 64-bit target for dynamic_annotations will make this
> > > configuration easier to follow. We are planning to merge the generic
targets
> > > with win64 targets at some stage, and having a dynamic_annotations64
target
> > will
> > > make it easier to do the right thing.
> > 
> > http://codereview.chromium.org/1992005/diff/15001/16004
> > File base/dynamic_annotations.h (right):
> > 
> > http://codereview.chromium.org/1992005/diff/15001/16004#newcode11
> > base/dynamic_annotations.h:11: #include
> > "base/third_party/dynamic_annotations/dynamic_annotations.h"
> > We already have
> > #include "base/dynamic_annotations.h"
> > lines in quite a few files in Chromium and in many other projects like
> > perftools, NaCL, etc.
> > The long line is much harder to remember.
> > 
> > That's why I'd rather stay with the short line.
> > On 2010/05/11 17:31:48, gregoryd wrote:
> > > This makes following the dependencies difficult. Can we just use the full
> path
> > > wherever this files is included?
> > 
> > http://codereview.chromium.org/1992005/diff/15001/16005
> > File base/third_party/dynamic_annotations/dynamic_annotations.c (right):
> > 
> > http://codereview.chromium.org/1992005/diff/15001/16005#newcode41
> > base/third_party/dynamic_annotations/dynamic_annotations.c:41: #include
> > "dynamic_annotations.h"
> > On 2010/05/11 17:31:48, gregoryd wrote:
> > > Please provide full path
> > 
> > Done.
> > 
> > http://codereview.chromium.org/1992005/diff/15001/16006
> > File base/third_party/dynamic_annotations/dynamic_annotations.h (right):
> > 
> > http://codereview.chromium.org/1992005/diff/15001/16006#newcode53
> > base/third_party/dynamic_annotations/dynamic_annotations.h:53: #ifndef
> > __DYNAMIC_ANNOTATIONS_H__
> > This guard is copied from the upstream version of dynamic_annotations.h
> > and I'm consistent with how the guard is done in valgrind.h:
> >
>
http://src.chromium.org/viewvc/chrome/trunk/src/base/third_party/valgrind/val...
> > On 2010/05/11 17:31:48, gregoryd wrote:
> > > The include guard should be
> > > BASE_THIRD_PARTY_DYNAMIC_ANNOTATIONS_DYNAMIC_ANNOTATIONS_H
> > 
> > http://codereview.chromium.org/1992005/diff/15001/16006#newcode115
> > base/third_party/dynamic_annotations/dynamic_annotations.h:115: /* Report
that
> > we are about to signal_all on the condition variable at "cv". */
> > On 2010/05/11 17:31:48, gregoryd wrote:
> > > long line
> > 
> > Done.
> > 
> > http://codereview.chromium.org/1992005/diff/15001/16006#newcode189
> > base/third_party/dynamic_annotations/dynamic_annotations.h:189: /* Report
that
> > we've just got an element from a FIFO queue at address "pcq". */
> > On 2010/05/11 17:31:48, gregoryd wrote:
> > > long line
> > 
> > Done.

[Timur Iskhodzhanov, 2010-05-12 14:11:01]

Should I do anything else to the CL ?

[gregoryd, 2010-05-12 17:28:50]

On 2010/05/12 14:11:01, Timur Iskhodzhanov wrote:
> Should I do anything else to the CL ?

I still think that base/dynamic_anotations.h should be removed. May be you can
get an additional opinion from developers who are more familiar with Chrome
conventions and style guidelines.

[Timur Iskhodzhanov, 2010-05-12 18:32:35]

Hi Evan, Will,

Can you please take a look at the CL?

Thank you,
Timur Iskhodzhanov

[Lei Zhang, 2010-05-13 00:44:10]

Looks like you'll need a LICENSE and README.Chromium file in order to put stuff
in third_party. Otherwise LGTM

[willchan no longer on Chromium, 2010-05-13 00:53:44]

I defer to thestig here.

On 2010/05/13 00:44:10, Lei Zhang wrote:
> Looks like you'll need a LICENSE and README.Chromium file in order to put
stuff
> in third_party. Otherwise LGTM

[Evan Martin, 2010-05-13 06:40:23]

Can you verify "tools/licenses.py scan" doesn't complain about this?

http://codereview.chromium.org/1992005/diff/27001/28004
File base/dynamic_annotations.h (right):

http://codereview.chromium.org/1992005/diff/27001/28004#newcode7
base/dynamic_annotations.h:7: // with a long filepath.
Is this really necessary?  It seems this header shouldn't be included from too
many places, so we could just fix the users.

[Timur Iskhodzhanov, 2010-05-13 13:00:34]

Alright, I've added README.chromium, LICENSE files
and removed base/dynamic_annotations.h and updated all references to the header

Please take another look

[Evan Martin, 2010-05-13 13:12:53]

The licensing bits look good to me.  I don't really understand the code change.

[willchan no longer on Chromium, 2010-05-13 16:45:14]

I think I mentioned this on your other email thread, but please make
sure that the code still builds with linux_use_tcmalloc=0.

On Thu, May 13, 2010 at 6:12 AM,  <evan@chromium.org> wrote:
> The licensing bits look good to me.  I don't really understand the code
> change.
>
> http://codereview.chromium.org/1992005/show
>

[gregoryd, 2010-05-13 16:51:48]

http://codereview.chromium.org/1992005/diff/37001/38001
File base/allocator/allocator.gyp (right):

http://codereview.chromium.org/1992005/diff/37001/38001#newcode62
base/allocator/allocator.gyp:62:
'<(tcmalloc_dir)/src/base/dynamic_annotations.h',
is this the right path?

[Alexander Potapenko, 2010-05-13 16:53:18]

On 2010/05/13 16:45:14, willchan wrote:
> I think I mentioned this on your other email thread, but please make
> sure that the code still builds with linux_use_tcmalloc=0.

Hint: this can be done on the Valgrind trybot.

[Timur Iskhodzhanov, 2010-05-13 19:05:49]

Will, Alexander,
As you can see I've already tested it on the linux_tsan trybot which has the
same GYP flags as linux_valgrind.

However, I'll double-check it locally w/o TCMalloc now.

http://codereview.chromium.org/1992005/diff/37001/38001
File base/allocator/allocator.gyp (right):

http://codereview.chromium.org/1992005/diff/37001/38001#newcode62
base/allocator/allocator.gyp:62:
'<(tcmalloc_dir)/src/base/dynamic_annotations.h',
Yes it is - tcmalloc has its own base/dynamic_annotations.{h,cc}
and they are used

As I've mentioned in the comment, we don't list dynamic_annotations.cc in this
gyp file since it is a copy of chromium/base/dynamic_annotations.cc (to be a bit
more precise - it is an older version; tcmalloc will be updated soon). To avoid
multiple symbols definitions, we've created a separate target
'dynamic_annotations' in base/base.gypi
On 2010/05/13 16:51:48, gregoryd wrote:
> is this the right path?

[willchan no longer on Chromium, 2010-05-13 19:13:43]

On Thu, May 13, 2010 at 12:05 PM,  <timurrrr@chromium.org> wrote:
> Will, Alexander,
> As you can see I've already tested it on the linux_tsan trybot which has the
> same GYP flags as linux_valgrind.

Ah, I didn't know that.  Sounds great then.  I just wanted to remind
you in case you had forgotten :)

>
> However, I'll double-check it locally w/o TCMalloc now.
>
>
> http://codereview.chromium.org/1992005/diff/37001/38001
> File base/allocator/allocator.gyp (right):
>
> http://codereview.chromium.org/1992005/diff/37001/38001#newcode62
> base/allocator/allocator.gyp:62:
> '<(tcmalloc_dir)/src/base/dynamic_annotations.h',
> Yes it is - tcmalloc has its own base/dynamic_annotations.{h,cc}
> and they are used
>
> As I've mentioned in the comment, we don't list dynamic_annotations.cc
> in this gyp file since it is a copy of
> chromium/base/dynamic_annotations.cc (to be a bit more precise - it is
> an older version; tcmalloc will be updated soon). To avoid multiple
> symbols definitions, we've created a separate target
> 'dynamic_annotations' in base/base.gypi
> On 2010/05/13 16:51:48, gregoryd wrote:
>>
>> is this the right path?
>
> http://codereview.chromium.org/1992005/show
>

[Timur Iskhodzhanov, 2010-05-13 19:19:16]

I did actually, but Alexander helped me make up for it :-)
On 2010/05/13 19:13:43, willchan wrote:
> On Thu, May 13, 2010 at 12:05 PM,  <mailto:timurrrr@chromium.org> wrote:
> > Will, Alexander,
> > As you can see I've already tested it on the linux_tsan trybot which has the
> > same GYP flags as linux_valgrind.
> 
> Ah, I didn't know that.  Sounds great then.  I just wanted to remind
> you in case you had forgotten :)
> 
> >
> > However, I'll double-check it locally w/o TCMalloc now.
> >
> >
> > http://codereview.chromium.org/1992005/diff/37001/38001
> > File base/allocator/allocator.gyp (right):
> >
> > http://codereview.chromium.org/1992005/diff/37001/38001#newcode62
> > base/allocator/allocator.gyp:62:
> > '<(tcmalloc_dir)/src/base/dynamic_annotations.h',
> > Yes it is - tcmalloc has its own base/dynamic_annotations.{h,cc}
> > and they are used
> >
> > As I've mentioned in the comment, we don't list dynamic_annotations.cc
> > in this gyp file since it is a copy of
> > chromium/base/dynamic_annotations.cc (to be a bit more precise - it is
> > an older version; tcmalloc will be updated soon). To avoid multiple
> > symbols definitions, we've created a separate target
> > 'dynamic_annotations' in base/base.gypi
> > On 2010/05/13 16:51:48, gregoryd wrote:
> >>
> >> is this the right path?
> >
> > http://codereview.chromium.org/1992005/show
> >
>

[Timur Iskhodzhanov, 2010-05-13 20:02:45]

Anything else I should do?
// If no - I commit once my no-tcmalloc client builds

On 2010/05/13 19:19:16, Timur Iskhodzhanov wrote:
> I did actually, but Alexander helped me make up for it :-)
> On 2010/05/13 19:13:43, willchan wrote:
> > On Thu, May 13, 2010 at 12:05 PM,  <mailto:timurrrr@chromium.org> wrote:
> > > Will, Alexander,
> > > As you can see I've already tested it on the linux_tsan trybot which has
the
> > > same GYP flags as linux_valgrind.
> > 
> > Ah, I didn't know that.  Sounds great then.  I just wanted to remind
> > you in case you had forgotten :)
> > 
> > >
> > > However, I'll double-check it locally w/o TCMalloc now.
> > >
> > >
> > > http://codereview.chromium.org/1992005/diff/37001/38001
> > > File base/allocator/allocator.gyp (right):
> > >
> > > http://codereview.chromium.org/1992005/diff/37001/38001#newcode62
> > > base/allocator/allocator.gyp:62:
> > > '<(tcmalloc_dir)/src/base/dynamic_annotations.h',
> > > Yes it is - tcmalloc has its own base/dynamic_annotations.{h,cc}
> > > and they are used
> > >
> > > As I've mentioned in the comment, we don't list dynamic_annotations.cc
> > > in this gyp file since it is a copy of
> > > chromium/base/dynamic_annotations.cc (to be a bit more precise - it is
> > > an older version; tcmalloc will be updated soon). To avoid multiple
> > > symbols definitions, we've created a separate target
> > > 'dynamic_annotations' in base/base.gypi
> > > On 2010/05/13 16:51:48, gregoryd wrote:
> > >>
> > >> is this the right path?
> > >
> > > http://codereview.chromium.org/1992005/show
> > >
> >

[Timur Iskhodzhanov, 2010-05-13 20:23:40]

No-TCMalloc local build succeded

On 2010/05/13 20:02:45, Timur Iskhodzhanov wrote:
> Anything else I should do?
> // If no - I commit once my no-tcmalloc client builds
> 
> On 2010/05/13 19:19:16, Timur Iskhodzhanov wrote:
> > I did actually, but Alexander helped me make up for it :-)
> > On 2010/05/13 19:13:43, willchan wrote:
> > > On Thu, May 13, 2010 at 12:05 PM,  <mailto:timurrrr@chromium.org> wrote:
> > > > Will, Alexander,
> > > > As you can see I've already tested it on the linux_tsan trybot which has
> the
> > > > same GYP flags as linux_valgrind.
> > > 
> > > Ah, I didn't know that.  Sounds great then.  I just wanted to remind
> > > you in case you had forgotten :)
> > > 
> > > >
> > > > However, I'll double-check it locally w/o TCMalloc now.
> > > >
> > > >
> > > > http://codereview.chromium.org/1992005/diff/37001/38001
> > > > File base/allocator/allocator.gyp (right):
> > > >
> > > > http://codereview.chromium.org/1992005/diff/37001/38001#newcode62
> > > > base/allocator/allocator.gyp:62:
> > > > '<(tcmalloc_dir)/src/base/dynamic_annotations.h',
> > > > Yes it is - tcmalloc has its own base/dynamic_annotations.{h,cc}
> > > > and they are used
> > > >
> > > > As I've mentioned in the comment, we don't list dynamic_annotations.cc
> > > > in this gyp file since it is a copy of
> > > > chromium/base/dynamic_annotations.cc (to be a bit more precise - it is
> > > > an older version; tcmalloc will be updated soon). To avoid multiple
> > > > symbols definitions, we've created a separate target
> > > > 'dynamic_annotations' in base/base.gypi
> > > > On 2010/05/13 16:51:48, gregoryd wrote:
> > > >>
> > > >> is this the right path?
> > > >
> > > > http://codereview.chromium.org/1992005/show
> > > >
> > >

[willchan no longer on Chromium, 2010-05-13 20:24:35]

Sounds good, go for it.

On Thu, May 13, 2010 at 1:23 PM,  <timurrrr@chromium.org> wrote:
> No-TCMalloc local build succeded
>
> On 2010/05/13 20:02:45, Timur Iskhodzhanov wrote:
>>
>> Anything else I should do?
>> // If no - I commit once my no-tcmalloc client builds
>
>> On 2010/05/13 19:19:16, Timur Iskhodzhanov wrote:
>> > I did actually, but Alexander helped me make up for it :-)
>> > On 2010/05/13 19:13:43, willchan wrote:
>> > > On Thu, May 13, 2010 at 12:05 PM,  <mailto:timurrrr@chromium.org>
>> > > wrote:
>> > > > Will, Alexander,
>> > > > As you can see I've already tested it on the linux_tsan trybot which
>> > > > has
>> the
>> > > > same GYP flags as linux_valgrind.
>> > >
>> > > Ah, I didn't know that.  Sounds great then.  I just wanted to remind
>> > > you in case you had forgotten :)
>> > >
>> > > >
>> > > > However, I'll double-check it locally w/o TCMalloc now.
>> > > >
>> > > >
>> > > > http://codereview.chromium.org/1992005/diff/37001/38001
>> > > > File base/allocator/allocator.gyp (right):
>> > > >
>> > > > http://codereview.chromium.org/1992005/diff/37001/38001#newcode62
>> > > > base/allocator/allocator.gyp:62:
>> > > > '<(tcmalloc_dir)/src/base/dynamic_annotations.h',
>> > > > Yes it is - tcmalloc has its own base/dynamic_annotations.{h,cc}
>> > > > and they are used
>> > > >
>> > > > As I've mentioned in the comment, we don't list
>> > > > dynamic_annotations.cc
>> > > > in this gyp file since it is a copy of
>> > > > chromium/base/dynamic_annotations.cc (to be a bit more precise - it
>> > > > is
>> > > > an older version; tcmalloc will be updated soon). To avoid multiple
>> > > > symbols definitions, we've created a separate target
>> > > > 'dynamic_annotations' in base/base.gypi
>> > > > On 2010/05/13 16:51:48, gregoryd wrote:
>> > > >>
>> > > >> is this the right path?
>> > > >
>> > > > http://codereview.chromium.org/1992005/show
>> > > >
>> > >
>
>
>
> http://codereview.chromium.org/1992005/show
>

[gregoryd, 2010-05-13 20:27:50]

LGTM