David Moore | 14 Jul 19:51 2010

static initializers

On linux we have approximately 1200 modules with at least one static initializer (there may be more than one such initializer in each module). The bulk are explained by the inclusion of header files, leaving around 200 that come from using static variables whose values can't be determined at compile time. Somewhere between 50 and 100 are in chromium code proper, with the rest coming from various third party libraries. Usage of these goes against our style guidelines.

Here's the details:

I started investigating this because on ChromeOS with current hardware we spend about 400ms getting to main() of Chrome. This is because about 26MB needs to be paged in. The data all shows that we're pinned I/O wise during this time, and it fits the read speed of about 70MB/s that we see on the hardware we're testing with. I believe that a significant part of this to come from the code and data paging necessary to run the static initializers, although I'm not sure yet just how much. We have a solution at the OS level that will allow this data to be read during boot without slowing down other boot tasks, but even then less data to read is better.

A few uses / patterns explained a significant percentage:
  1. Including <iostream>
    This file has this in it:
    // For construction of filebuffers for cout, cin, cerr, clog et. al.
    static ios_base::Init __ioinit;
    I believe this is to allow people to use buffered I/O within their own static initializers. But any file that includes iostream in any way gets a static initializer. Commenting this out reduced the number of files with at least one to about 600, and the data read to 24.5MB.

  2. Including any header that contains a use of scoped_ptr_malloc<>
    We have 9 uses in header files. This template includes this declaration:
  3. static FreeProc const free_;
    When I remove this declaration (and I'm sure break much of chrome) it further reduces the number of files with at least one static initializer to about 225, and the data read to 19.5MB.

  4. Statically declaring a SkColor variable with the result of SkColorSetRGB() or SkColorSetARGB()
    Even though SkColorSetARGB() is declared as inline the compiler still generates a static initializer. If I replace it with a macro it doesn't. That reduces the files with at least one to about 200, with little affect on the amount of data read.
I've spot checked the remaining 200 and they appear to fall into a few camps:
  1. Using std::string or std::wstring when we should be using char or wchar_t for string constants.
  2. Using a static class instance of another kind as a variable, when it should instead be contained within a static function with a local static variable
  3. Using a static scaler but setting its value to the result of a function call that can't be computed at compile time, such as string16.
I've attached the list of these files. This list only has the filenames and includes third party things as well.

Here's what I think we should do:
  1. Find solutions to the 3 patterns above. Perhaps we're including these things way more aggressively than we need to. Or maybe there's a way to avoid the statics in the first place.
  2. Fix the remaining things. Most of the changes (at least w/in the chromium source) are straightforward. I can do it, or the people who know the code better can.
  3. Get better about looking for these things in code reviews and reject any changes that contain code that produces static initializers. I believe webkit already does this.
  4. Add tools to the perfbots that can track the number of static initializers that we have in the code. I haven't looked at the Windows or Mac side of this, but it's relatively easy to find them in the ELF format, using readelf and looking for the .ctors section.
Dave

--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
Attachment (ctors): application/octet-stream, 4721 bytes
Peter Kasting | 14 Jul 19:57 2010
Picon

Re: static initializers

On Wed, Jul 14, 2010 at 10:51 AM, David Moore <davemoore <at> chromium.org> wrote:
  1. Get better about looking for these things in code reviews and reject any changes that contain code that produces static initializers. I believe webkit already does this.
I believe WebKit actually won't compile if you add these, although I'm not sure.

PK

--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
Amit Joshi | 14 Jul 19:59 2010

Re: static initializers

Tracking bug with some more information: http://code.google.com/p/chromium/issues/detail?id=48097


On Wed, Jul 14, 2010 at 10:57 AM, Peter Kasting <pkasting <at> google.com> wrote:
On Wed, Jul 14, 2010 at 10:51 AM, David Moore <davemoore <at> chromium.org> wrote:
  1. Get better about looking for these things in code reviews and reject any changes that contain code that produces static initializers. I believe webkit already does this.
I believe WebKit actually won't compile if you add these, although I'm not sure.

PK

--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
Evan Martin | 14 Jul 20:17 2010

Re: static initializers

On Wed, Jul 14, 2010 at 10:57 AM, Peter Kasting <pkasting <at> google.com> wrote:
> On Wed, Jul 14, 2010 at 10:51 AM, David Moore <davemoore <at> chromium.org>
> wrote:
>>
>> Get better about looking for these things in code reviews and reject any
>> changes that contain code that produces static initializers. I believe
>> webkit already does this.
>
> I believe WebKit actually won't compile if you add these, although I'm not
> sure.

http://trac.webkit.org/browser/trunk/JavaScriptCore/wtf/StaticConstructors.h
is all I could find.  It provides a macro that is used in place of
statics that has the effect of a static without an initializer being
involved.

24	// We need to avoid having static constructors. We achieve this
25	// with two separate methods for GCC and MSVC. Both methods prevent
the static
26	// initializers from being registered and called on program
startup. On GCC, we
27	// declare the global objects with a different type that can be POD default
28	// initialized by the linker/loader. On MSVC we use a special
compiler feature
29	// to have the CRT ignore our static initializers. The constructors
will never
30	// be called and the objects will be left uninitialized.
31	//
32	// With both of these approaches, we must define and explicitly call an init
33	// routine that uses placement new to create the objects and overwrite the
34	// uninitialized placeholders.
35	//
36	// This is not completely portable, but is what we have for now without
37	// changing how a lot of code accesses these global objects.

--

-- 
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe: 
    http://groups.google.com/a/chromium.org/group/chromium-dev

Adam Barth | 14 Jul 20:30 2010

Re: static initializers

On Wed, Jul 14, 2010 at 11:17 AM, Evan Martin <evan <at> chromium.org> wrote:
> On Wed, Jul 14, 2010 at 10:57 AM, Peter Kasting <pkasting <at> google.com> wrote:
>> On Wed, Jul 14, 2010 at 10:51 AM, David Moore <davemoore <at> chromium.org>
>> wrote:
>>>
>>> Get better about looking for these things in code reviews and reject any
>>> changes that contain code that produces static initializers. I believe
>>> webkit already does this.
>>
>> I believe WebKit actually won't compile if you add these, although I'm not
>> sure.
>
> http://trac.webkit.org/browser/trunk/JavaScriptCore/wtf/StaticConstructors.h
> is all I could find.  It provides a macro that is used in place of
> statics that has the effect of a static without an initializer being
> involved.

There's also a step in the build (at least on mac) that checks for
static initializers and errors out if any are present.

Adam

--

-- 
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe: 
    http://groups.google.com/a/chromium.org/group/chromium-dev

Adam Barth | 14 Jul 20:34 2010

Re: static initializers

On Wed, Jul 14, 2010 at 11:30 AM, Adam Barth <abarth <at> chromium.org> wrote:
> On Wed, Jul 14, 2010 at 11:17 AM, Evan Martin <evan <at> chromium.org> wrote:
>> On Wed, Jul 14, 2010 at 10:57 AM, Peter Kasting <pkasting <at> google.com> wrote:
>>> On Wed, Jul 14, 2010 at 10:51 AM, David Moore <davemoore <at> chromium.org>
>>> wrote:
>>>>
>>>> Get better about looking for these things in code reviews and reject any
>>>> changes that contain code that produces static initializers. I believe
>>>> webkit already does this.
>>>
>>> I believe WebKit actually won't compile if you add these, although I'm not
>>> sure.
>>
>> http://trac.webkit.org/browser/trunk/JavaScriptCore/wtf/StaticConstructors.h
>> is all I could find.  It provides a macro that is used in place of
>> statics that has the effect of a static without an initializer being
>> involved.
>
> There's also a step in the build (at least on mac) that checks for
> static initializers and errors out if any are present.

I could be wrong, but I think it just runs this script:

http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/check-for-global-initializers

Adam

--

-- 
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe: 
    http://groups.google.com/a/chromium.org/group/chromium-dev

Antoine Labour | 14 Jul 20:06 2010

Re: [cros-dev] static initializers



On Wed, Jul 14, 2010 at 10:51 AM, David Moore <davemoore <at> chromium.org> wrote:
On linux we have approximately 1200 modules with at least one static initializer (there may be more than one such initializer in each module). The bulk are explained by the inclusion of header files, leaving around 200 that come from using static variables whose values can't be determined at compile time. Somewhere between 50 and 100 are in chromium code proper, with the rest coming from various third party libraries. Usage of these goes against our style guidelines.

Here's the details:

I started investigating this because on ChromeOS with current hardware we spend about 400ms getting to main() of Chrome. This is because about 26MB needs to be paged in. The data all shows that we're pinned I/O wise during this time, and it fits the read speed of about 70MB/s that we see on the hardware we're testing with. I believe that a significant part of this to come from the code and data paging necessary to run the static initializers, although I'm not sure yet just how much. We have a solution at the OS level that will allow this data to be read during boot without slowing down other boot tasks, but even then less data to read is better.

A few uses / patterns explained a significant percentage:
  1. Including <iostream>
    This file has this in it:
    // For construction of filebuffers for cout, cin, cerr, clog et. al.
    static ios_base::Init __ioinit;
    I believe this is to allow people to use buffered I/O within their own static initializers. But any file that includes iostream in any way gets a static initializer. Commenting this out reduced the number of files with at least one to about 600, and the data read to 24.5MB.

  2. Including any header that contains a use of scoped_ptr_malloc<>
    We have 9 uses in header files. This template includes this declaration:
  3. static FreeProc const free_;
    When I remove this declaration (and I'm sure break much of chrome) it further reduces the number of files with at least one static initializer to about 225, and the data read to 19.5MB.
I understand that it generates data, but why is this one generating initializers ? It's a POD, it should be resolved at link time. What does the initializer look like ?

  1. Statically declaring a SkColor variable with the result of SkColorSetRGB() or SkColorSetARGB()
    Even though SkColorSetARGB() is declared as inline the compiler still generates a static initializer. If I replace it with a macro it doesn't. That reduces the files with at least one to about 200, with little affect on the amount of data read.

Have you tried marking those 2 functions const and see if it helps ?

Antoine
 
I've spot checked the remaining 200 and they appear to fall into a few camps:
  1. Using std::string or std::wstring when we should be using char or wchar_t for string constants.
  2. Using a static class instance of another kind as a variable, when it should instead be contained within a static function with a local static variable
  3. Using a static scaler but setting its value to the result of a function call that can't be computed at compile time, such as string16.
I've attached the list of these files. This list only has the filenames and includes third party things as well.

Here's what I think we should do:
  1. Find solutions to the 3 patterns above. Perhaps we're including these things way more aggressively than we need to. Or maybe there's a way to avoid the statics in the first place.
  2. Fix the remaining things. Most of the changes (at least w/in the chromium source) are straightforward. I can do it, or the people who know the code better can.
  3. Get better about looking for these things in code reviews and reject any changes that contain code that produces static initializers. I believe webkit already does this.
  4. Add tools to the perfbots that can track the number of static initializers that we have in the code. I haven't looked at the Windows or Mac side of this, but it's relatively easy to find them in the ELF format, using readelf and looking for the .ctors section.
Dave

--
Chromium OS Developers mailing list: chromium-os-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-os-dev?hl=en

--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
David Moore | 14 Jul 22:03 2010

Re: [cros-dev] static initializers

Adding const didn't change anything. I've reduced it to all code in one file that looks like this:


typedef unsigned int SkColor;
typedef unsigned char uint8_t;

static const inline SkColor SkColorSetARGB(uint8_t a, uint8_t r, uint8_t g, uint8_t b) {
  return (a << 24) | (r << 16) | (g << 8) | (b << 0);
}

static const SkColor foo = SkColorSetARGB(0, 1, 2, 3);

The initializer is:
   0x080e5342 <+0>:     push   %ebp
   0x080e5343 <+1>:     mov    %esp,%ebp
   0x080e5345 <+3>:     mov    $0xffff,%edx
   0x080e534a <+8>:     mov    $0x1,%eax
   0x080e534f <+13>:    pop    %ebp
   0x080e5350 <+14>:    jmp    0x80e5320 <_Z41__static_initialization_and_destruction_0ii>

which jumps to this:
   0x080e5320 <+0>:     push   %ebp
   0x080e5321 <+1>:     mov    %esp,%ebp
   0x080e5323 <+3>:     sub    $0x1,%eax
   0x080e5326 <+6>:     je     0x80e532a <_Z41__static_initialization_and_destruction_0ii+10>
   0x080e5328 <+8>:     pop    %ebp
   0x080e5329 <+9>:     ret
   0x080e532a <+10>:    cmp    $0xffff,%edx
   0x080e5330 <+16>:    jne    0x80e5328 <_Z41__static_initialization_and_destruction_0ii+8>
   0x080e5332 <+18>:    movl   $0x10203,0xa8c22c4
   0x080e533c <+28>:    pop    %ebp
   0x080e533d <+29>:    lea    0x0(%esi),%esi
   0x080e5340 <+32>:    ret

All the static initializers have that bit of prologue...I guess they're guarding other code jumping to them for some reason.

If use this instead 
#define SkColorSetARGB(a, r, g, b) \
  static_cast<SkColor>((a << 24) | (r << 16) | (g << 8) | (b << 0))

I get no static initializer.

Dave

On Wed, Jul 14, 2010 at 11:06 AM, Antoine Labour <piman <at> chromium.org> wrote:


On Wed, Jul 14, 2010 at 10:51 AM, David Moore <davemoore <at> chromium.org> wrote:
On linux we have approximately 1200 modules with at least one static initializer (there may be more than one such initializer in each module). The bulk are explained by the inclusion of header files, leaving around 200 that come from using static variables whose values can't be determined at compile time. Somewhere between 50 and 100 are in chromium code proper, with the rest coming from various third party libraries. Usage of these goes against our style guidelines.

Here's the details:

I started investigating this because on ChromeOS with current hardware we spend about 400ms getting to main() of Chrome. This is because about 26MB needs to be paged in. The data all shows that we're pinned I/O wise during this time, and it fits the read speed of about 70MB/s that we see on the hardware we're testing with. I believe that a significant part of this to come from the code and data paging necessary to run the static initializers, although I'm not sure yet just how much. We have a solution at the OS level that will allow this data to be read during boot without slowing down other boot tasks, but even then less data to read is better.

A few uses / patterns explained a significant percentage:
  1. Including <iostream>
    This file has this in it:
    // For construction of filebuffers for cout, cin, cerr, clog et. al.
    static ios_base::Init __ioinit;
    I believe this is to allow people to use buffered I/O within their own static initializers. But any file that includes iostream in any way gets a static initializer. Commenting this out reduced the number of files with at least one to about 600, and the data read to 24.5MB.

  2. Including any header that contains a use of scoped_ptr_malloc<>
    We have 9 uses in header files. This template includes this declaration:
  3. static FreeProc const free_;
    When I remove this declaration (and I'm sure break much of chrome) it further reduces the number of files with at least one static initializer to about 225, and the data read to 19.5MB.
I understand that it generates data, but why is this one generating initializers ? It's a POD, it should be resolved at link time. What does the initializer look like ?

  1. Statically declaring a SkColor variable with the result of SkColorSetRGB() or SkColorSetARGB()
    Even though SkColorSetARGB() is declared as inline the compiler still generates a static initializer. If I replace it with a macro it doesn't. That reduces the files with at least one to about 200, with little affect on the amount of data read.

Have you tried marking those 2 functions const and see if it helps ?

Antoine
 
I've spot checked the remaining 200 and they appear to fall into a few camps:
  1. Using std::string or std::wstring when we should be using char or wchar_t for string constants.
  2. Using a static class instance of another kind as a variable, when it should instead be contained within a static function with a local static variable
  3. Using a static scaler but setting its value to the result of a function call that can't be computed at compile time, such as string16.
I've attached the list of these files. This list only has the filenames and includes third party things as well.

Here's what I think we should do:
  1. Find solutions to the 3 patterns above. Perhaps we're including these things way more aggressively than we need to. Or maybe there's a way to avoid the statics in the first place.
  2. Fix the remaining things. Most of the changes (at least w/in the chromium source) are straightforward. I can do it, or the people who know the code better can.
  3. Get better about looking for these things in code reviews and reject any changes that contain code that produces static initializers. I believe webkit already does this.
  4. Add tools to the perfbots that can track the number of static initializers that we have in the code. I haven't looked at the Windows or Mac side of this, but it's relatively easy to find them in the ELF format, using readelf and looking for the .ctors section.
Dave

--
Chromium OS Developers mailing list: chromium-os-dev <at> chromium.org

View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-os-dev?hl=en


--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
Antoine Labour | 14 Jul 22:22 2010

Re: [cros-dev] static initializers



On Wed, Jul 14, 2010 at 1:03 PM, David Moore <davemoore <at> chromium.org> wrote:
Adding const didn't change anything. I've reduced it to all code in one file that looks like this:

typedef unsigned int SkColor;
typedef unsigned char uint8_t;

static const inline SkColor SkColorSetARGB(uint8_t a, uint8_t r, uint8_t g, uint8_t b) {
  return (a << 24) | (r << 16) | (g << 8) | (b << 0);
}

To make the function const, you need to declare as:

static inline SkColor SkColorSetARGB(uint8_t a, uint8_t r, uint8_t g, uint8_t b) const {
  return (a << 24) | (r << 16) | (g << 8) | (b << 0);
}

(in your version, const applies to SkColor so it doesn't do anything).
Not sure it'll help at all, and a macro is probably just as good, I'm just curious.

Antoine


static const SkColor foo = SkColorSetARGB(0, 1, 2, 3);

The initializer is:
   0x080e5342 <+0>:     push   %ebp
   0x080e5343 <+1>:     mov    %esp,%ebp
   0x080e5345 <+3>:     mov    $0xffff,%edx
   0x080e534a <+8>:     mov    $0x1,%eax
   0x080e534f <+13>:    pop    %ebp
   0x080e5350 <+14>:    jmp    0x80e5320 <_Z41__static_initialization_and_destruction_0ii>

which jumps to this:
   0x080e5320 <+0>:     push   %ebp
   0x080e5321 <+1>:     mov    %esp,%ebp
   0x080e5323 <+3>:     sub    $0x1,%eax
   0x080e5326 <+6>:     je     0x80e532a <_Z41__static_initialization_and_destruction_0ii+10>
   0x080e5328 <+8>:     pop    %ebp
   0x080e5329 <+9>:     ret
   0x080e532a <+10>:    cmp    $0xffff,%edx
   0x080e5330 <+16>:    jne    0x80e5328 <_Z41__static_initialization_and_destruction_0ii+8>
   0x080e5332 <+18>:    movl   $0x10203,0xa8c22c4
   0x080e533c <+28>:    pop    %ebp
   0x080e533d <+29>:    lea    0x0(%esi),%esi
   0x080e5340 <+32>:    ret

All the static initializers have that bit of prologue...I guess they're guarding other code jumping to them for some reason.

If use this instead 
#define SkColorSetARGB(a, r, g, b) \
  static_cast<SkColor>((a << 24) | (r << 16) | (g << 8) | (b << 0))

I get no static initializer.

Dave

On Wed, Jul 14, 2010 at 11:06 AM, Antoine Labour <piman <at> chromium.org> wrote:


On Wed, Jul 14, 2010 at 10:51 AM, David Moore <davemoore <at> chromium.org> wrote:
On linux we have approximately 1200 modules with at least one static initializer (there may be more than one such initializer in each module). The bulk are explained by the inclusion of header files, leaving around 200 that come from using static variables whose values can't be determined at compile time. Somewhere between 50 and 100 are in chromium code proper, with the rest coming from various third party libraries. Usage of these goes against our style guidelines.

Here's the details:

I started investigating this because on ChromeOS with current hardware we spend about 400ms getting to main() of Chrome. This is because about 26MB needs to be paged in. The data all shows that we're pinned I/O wise during this time, and it fits the read speed of about 70MB/s that we see on the hardware we're testing with. I believe that a significant part of this to come from the code and data paging necessary to run the static initializers, although I'm not sure yet just how much. We have a solution at the OS level that will allow this data to be read during boot without slowing down other boot tasks, but even then less data to read is better.

A few uses / patterns explained a significant percentage:
  1. Including <iostream>
    This file has this in it:
    // For construction of filebuffers for cout, cin, cerr, clog et. al.
    static ios_base::Init __ioinit;
    I believe this is to allow people to use buffered I/O within their own static initializers. But any file that includes iostream in any way gets a static initializer. Commenting this out reduced the number of files with at least one to about 600, and the data read to 24.5MB.

  2. Including any header that contains a use of scoped_ptr_malloc<>
    We have 9 uses in header files. This template includes this declaration:
  3. static FreeProc const free_;
    When I remove this declaration (and I'm sure break much of chrome) it further reduces the number of files with at least one static initializer to about 225, and the data read to 19.5MB.
I understand that it generates data, but why is this one generating initializers ? It's a POD, it should be resolved at link time. What does the initializer look like ?

  1. Statically declaring a SkColor variable with the result of SkColorSetRGB() or SkColorSetARGB()
    Even though SkColorSetARGB() is declared as inline the compiler still generates a static initializer. If I replace it with a macro it doesn't. That reduces the files with at least one to about 200, with little affect on the amount of data read.

Have you tried marking those 2 functions const and see if it helps ?

Antoine
 
I've spot checked the remaining 200 and they appear to fall into a few camps:
  1. Using std::string or std::wstring when we should be using char or wchar_t for string constants.
  2. Using a static class instance of another kind as a variable, when it should instead be contained within a static function with a local static variable
  3. Using a static scaler but setting its value to the result of a function call that can't be computed at compile time, such as string16.
I've attached the list of these files. This list only has the filenames and includes third party things as well.

Here's what I think we should do:
  1. Find solutions to the 3 patterns above. Perhaps we're including these things way more aggressively than we need to. Or maybe there's a way to avoid the statics in the first place.
  2. Fix the remaining things. Most of the changes (at least w/in the chromium source) are straightforward. I can do it, or the people who know the code better can.
  3. Get better about looking for these things in code reviews and reject any changes that contain code that produces static initializers. I believe webkit already does this.
  4. Add tools to the perfbots that can track the number of static initializers that we have in the code. I haven't looked at the Windows or Mac side of this, but it's relatively easy to find them in the ELF format, using readelf and looking for the .ctors section.
Dave

--
Chromium OS Developers mailing list: chromium-os-dev <at> chromium.org

View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-os-dev?hl=en



--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
Peter Kasting | 14 Jul 22:25 2010
Picon

Re: Re: [cros-dev] static initializers

On Wed, Jul 14, 2010 at 1:22 PM, Antoine Labour <piman <at> chromium.org> wrote:
To make the function const, you need to declare as:

static inline SkColor SkColorSetARGB(uint8_t a, uint8_t r, uint8_t g, uint8_t b) const

This is an illegal declaration.  SkColoSetARGB() is not a member function and therefore cannot be declared const.

PK 

--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
Antoine Labour | 14 Jul 22:40 2010
Picon

Re: Re: [cros-dev] static initializers



On Wed, Jul 14, 2010 at 1:25 PM, Peter Kasting <pkasting <at> google.com> wrote:
On Wed, Jul 14, 2010 at 1:22 PM, Antoine Labour <piman <at> chromium.org> wrote:
To make the function const, you need to declare as:

static inline SkColor SkColorSetARGB(uint8_t a, uint8_t r, uint8_t g, uint8_t b) const

This is an illegal declaration.  SkColoSetARGB() is not a member function and therefore cannot be declared const.

PK 

My bad, that's not valid in C++. I thought it was the way to declare it as pure in c99, and that they kept that in C++, the latter is definitely not the case and I can't find a reference to the former. So ignore me.

Antoine

--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
Jochen Eisinger | 14 Jul 22:59 2010

Re: Re: [cros-dev] static initializers

On Wed, Jul 14, 2010 at 10:40 PM, Antoine Labour <piman <at> google.com> wrote:
>
>
> On Wed, Jul 14, 2010 at 1:25 PM, Peter Kasting <pkasting <at> google.com> wrote:
>>
>> On Wed, Jul 14, 2010 at 1:22 PM, Antoine Labour <piman <at> chromium.org>
>> wrote:
>>>
>>> To make the function const, you need to declare as:
>>>
>>> static inline SkColor SkColorSetARGB(uint8_t a, uint8_t r, uint8_t g,
>>> uint8_t b) const
>>
>> This is an illegal declaration.  SkColoSetARGB() is not a member function
>> and therefore cannot be declared const.
>> PK
>
> My bad, that's not valid in C++. I thought it was the way to declare it as
> pure in c99, and that they kept that in C++, the latter is definitely not
> the case and I can't find a reference to the former. So ignore me.
> Antoine

gcc supports the function attributes const and pure (const being pure
and additionally does not access global objects). If it would help to
reduce the amount of static initializers, we could add a macro
CONST_FUNCTION or similar that would evaluate to
__attribute__((const)) if compiled with gcc and to nothing otherwise.

>
> --
> Chromium Developers mailing list: chromium-dev <at> chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev
>

--

-- 
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe: 
    http://groups.google.com/a/chromium.org/group/chromium-dev

Jochen Eisinger | 14 Jul 23:06 2010

Re: Re: [cros-dev] static initializers

[this time from the correct email address]

On Wed, Jul 14, 2010 at 10:40 PM, Antoine Labour <piman <at> google.com> wrote:
>
>
> On Wed, Jul 14, 2010 at 1:25 PM, Peter Kasting <pkasting <at> google.com> wrote:
>>
>> On Wed, Jul 14, 2010 at 1:22 PM, Antoine Labour <piman <at> chromium.org>
>> wrote:
>>>
>>> To make the function const, you need to declare as:
>>>
>>> static inline SkColor SkColorSetARGB(uint8_t a, uint8_t r, uint8_t g,
>>> uint8_t b) const
>>
>> This is an illegal declaration.  SkColoSetARGB() is not a member function
>> and therefore cannot be declared const.
>> PK
>
> My bad, that's not valid in C++. I thought it was the way to declare it as
> pure in c99, and that they kept that in C++, the latter is definitely not
> the case and I can't find a reference to the former. So ignore me.
> Antoine

gcc supports the function attributes const and pure (const being pure
and additionally does not access global objects). If it would help to
reduce the amount of static initializers, we could add a macro
CONST_FUNCTION or similar that would evaluate to
__attribute__((const)) if compiled with gcc and to nothing otherwise.

>
> --
> Chromium Developers mailing list: chromium-dev <at> chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev
>

--

-- 
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe: 
    http://groups.google.com/a/chromium.org/group/chromium-dev

Jonathan Dixon | 14 Jul 23:18 2010

Re: static initializers



On 14 July 2010 18:51, David Moore <davemoore <at> chromium.org> wrote:
On linux we have approximately 1200 modules with at least one static initializer (there may be more than one such initializer in each module). The bulk are explained by the inclusion of header files, leaving around 200 that come from using static variables whose values can't be determined at compile time. Somewhere between 50 and 100 are in chromium code proper, with the rest coming from various third party libraries. Usage of these goes against our style guidelines.

Here's the details:

I started investigating this because on ChromeOS with current hardware we spend about 400ms getting to main() of Chrome. This is because about 26MB needs to be paged in. The data all shows that we're pinned I/O wise during this time, and it fits the read speed of about 70MB/s that we see on the hardware we're testing with. I believe that a significant part of this to come from the code and data paging necessary to run the static initializers, although I'm not sure yet just how much. We have a solution at the OS level that will allow this data to be read during boot without slowing down other boot tasks, but even then less data to read is better.

A few uses / patterns explained a significant percentage:
  1. Including <iostream>
    This file has this in it:
    // For construction of filebuffers for cout, cin, cerr, clog et. al.
    static ios_base::Init __ioinit;
    I believe this is to allow people to use buffered I/O within their own static initializers. But any file that includes iostream in any way gets a static initializer. Commenting this out reduced the number of files with at least one to about 600, and the data read to 24.5MB.

  2. Including any header that contains a use of scoped_ptr_malloc<>
    We have 9 uses in header files. This template includes this declaration:
  3. static FreeProc const free_;
    When I remove this declaration (and I'm sure break much of chrome) it further reduces the number of files with at least one static initializer to about 225, and the data read to 19.5MB.

My experience is scoped_ptr_malloc<> is a bit disappointing to use. Every time I have needed it, I just have a plain old static function I want to bind it to, yet I need to code up a functor class just to call it. It's even more disappointing to find this class has a knock-on cost on app startup time. Perhaps a version for use with a plain function would help (in both simplicity of use, and efficiency),

Alternatively, with the existing versions could we alter it to just instantiate the functor class instance when needed?
  ~scoped_ptr_malloc() {
-    free_(ptr_);
+    FreeProc()(ptr_);
 
}

this could have bad side-effects (as the functor will be instantiated more times). If so, maybe some use of Singleton<> could address that
 
 
  1. Statically declaring a SkColor variable with the result of SkColorSetRGB() or SkColorSetARGB()
    Even though SkColorSetARGB() is declared as inline the compiler still generates a static initializer. If I replace it with a macro it doesn't. That reduces the files with at least one to about 200, with little affect on the amount of data read.
I've spot checked the remaining 200 and they appear to fall into a few camps:
  1. Using std::string or std::wstring when we should be using char or wchar_t for string constants.
  2. Using a static class instance of another kind as a variable, when it should instead be contained within a static function with a local static variable
  3. Using a static scaler but setting its value to the result of a function call that can't be computed at compile time, such as string16.
I've attached the list of these files. This list only has the filenames and includes third party things as well.

Here's what I think we should do:
  1. Find solutions to the 3 patterns above. Perhaps we're including these things way more aggressively than we need to. Or maybe there's a way to avoid the statics in the first place.
  2. Fix the remaining things. Most of the changes (at least w/in the chromium source) are straightforward. I can do it, or the people who know the code better can.
  3. Get better about looking for these things in code reviews and reject any changes that contain code that produces static initializers. I believe webkit already does this.
  4. Add tools to the perfbots that can track the number of static initializers that we have in the code. I haven't looked at the Windows or Mac side of this, but it's relatively easy to find them in the ELF format, using readelf and looking for the .ctors section.
Dave

--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
Jonathan Dixon | 15 Jul 16:14 2010

Re: static initializers



On 14 July 2010 22:18, Jonathan Dixon <joth <at> chromium.org> wrote:
My experience is scoped_ptr_malloc<> is a bit disappointing to use. Every time I have needed it, I just have a plain old static function I want to bind it to, yet I need to code up a functor class just to call it. It's even more disappointing to find this class has a knock-on cost on app startup time. Perhaps a version for use with a plain function would help ...


In case it's any help, I started playing around with this idea: http://codereview.chromium.org/2804042

As I expected, this mostly results in deletion of functor-defining code, with a few tricks needed to adapt FreeXxx() functions that take void* or return something other than void. (My template meta foo is not the best; there maybe cleaner ways to handle some of these conversions)

This is incomplete, but has some merit. The biggest issue I see is the end result would need to be renamed to something other than scoped_ptr_malloc to avoid massive confusion with all the other copies of it that are out there. (Even within chrome we have half a dozen duplicate copies of this class)

--
Chromium Developers mailing list: chromium-dev <at> chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

Gmane