Get rid of a LazyInstance usage to get rid of code execution at static init time. This doesn't ne...

Get rid of a LazyInstance usage to get rid of code execution at static init time. This doesn't need to be thread safe.

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

[michaeln, 2011-08-31 21:41:33]

I'll have to fiddle with some suppressions for this one i think since we'll be
leaking the map.

[Evan Martin, 2011-08-31 21:42:58]

Is it possible to hang this off of the appropriate data structure?  For example,
in a multi-profile world is it correct for there to be one single app cache?

[michaeln, 2011-08-31 21:46:55]

On 2011/08/31 21:42:58, Evan Martin wrote:
> Is it possible to hang this off of the appropriate data structure?  For
example,
> in a multi-profile world is it correct for there to be one single app cache?

This is renderer-side stuff so multi-profile isn't an issue there.

[Evan Martin, 2011-08-31 22:35:35]

The LazyInstance pattern is supposed to work.  I wonder why it doesn't...

[michaeln, 2011-08-31 22:41:32]

On 2011/08/31 22:35:35, Evan Martin wrote:
> The LazyInstance pattern is supposed to work.  I wonder why it doesn't...

Actually, i'm not sure if the lazy instance is the reason for this file being
listed in the dump, the filename is listed with no symbols? I just saw something
with a ctor and sought to kill it.

[Evan Martin, 2011-08-31 22:49:54]

On 2011/08/31 22:41:32, michaeln wrote:
> On 2011/08/31 22:35:35, Evan Martin wrote:
> > The LazyInstance pattern is supposed to work.  I wonder why it doesn't...
> 
> Actually, i'm not sure if the lazy instance is the reason for this file being
> listed in the dump, the filename is listed with no symbols? I just saw
something
> with a ctor and sought to kill it.

The base::LINKER_INITIALIZED in there is supposed to tag that the object
explicitly doesn't do any static initialization (see the comments near its
definition)

[Evan Martin, 2011-08-31 22:51:39]

On 2011/08/31 22:49:54, Evan Martin wrote:
> On 2011/08/31 22:41:32, michaeln wrote:
> > On 2011/08/31 22:35:35, Evan Martin wrote:
> > > The LazyInstance pattern is supposed to work.  I wonder why it doesn't...
> > 
> > Actually, i'm not sure if the lazy instance is the reason for this file
being
> > listed in the dump, the filename is listed with no symbols? I just saw
> something
> > with a ctor and sought to kill it.
> 
> The base::LINKER_INITIALIZED in there is supposed to tag that the object
> explicitly doesn't do any static initialization (see the comments near its
> definition)

Heh, here's the body of the initializer for that function:

Dump of assembler code for function
_GLOBAL__I_web_application_cache_host_impl.cc(void):
   0x0000000000d20900 <+0>:	push   %rbp
   0x0000000000d20901 <+1>:	mov    %rsp,%rbp
   0x0000000000d20904 <+4>:	leaveq 
   0x0000000000d20905 <+5>:	retq   
End of assembler dump.


I think this is just the compiler being stupid.  I'll look into it more.

[michaeln, 2011-09-01 02:45:13]

Hmmm... fwiw, here's what dumpbin.exe says about the g_host_map LazyInstance in
a debug library.  I guess the notable thing is the .CRT stuff, sounds like crt
initializers. This sort of glop is not there when using the butt simple local
static ptr.

.CRT$XCU name
       0 physical address
       0 virtual address
       4 size of raw data
   695A3 file pointer to raw data (000695A3 to 000695A6)
   695A7 file pointer to relocation table
       0 file pointer to line numbers
       1 number of relocations
       0 number of line numbers
40300040 flags
         Initialized Data
         4 byte align
         Read Only

RAW DATA #4F1
  00000000: 00 00 00 00                                      ....

RELOCATIONS #4F1
                                                Symbol    Symbol
 Offset    Type              Applied To         Index     Name
 --------  ----------------  -----------------  --------  ------
 00000000  DIR32                      00000000       D86 
??__Eg_hosts_map@?A0xc934e6d3@appcache@@YAXXZ (void __cdecl appcache::`anonymous
namespace'::`dynamic initializer for 'g_hosts_map''(void))

D86 00000000 SECT4E2 notype ()    Static      |
??__Eg_hosts_map@?A0xc934e6d3@appcache@@YAXXZ (void __cdecl appcache::`anonymous
namespace'::`dynamic initializer for 'g_hosts_map''(void))

 00000A73  SECREL                     00000000       DA2  _g_hosts_map
 00000A77  SECTION                        0000       DA2  _g_hosts_map
 00000AB4  SECREL                     00000000       DAB 
_g_hosts_map$initializer$
 00000AB8  SECTION                        0000       DAB 
_g_hosts_map$initializer$

... and a bunch of other junk

[Evan Martin, 2011-09-01 17:04:06]

Interesting!  On my end I asked around and learned that newer gccs
don't emit the no-op function, so it's a bug in gcc of sorts.  I don't
know how to analyze Windows stuff too well, but if you could get a
dump of the assembly of the initializer code that would help us track
it down further.

On Wed, Aug 31, 2011 at 7:45 PM,  <michaeln@chromium.org> wrote:
> Hmmm... fwiw, here's what dumpbin.exe says about the g_host_map LazyInstance
> in
> a debug library.  I guess the notable thing is the .CRT stuff, sounds like
> crt
> initializers. This sort of glop is not there when using the butt simple
> local
> static ptr.
>
> .CRT$XCU name
>       0 physical address
>       0 virtual address
>       4 size of raw data
>   695A3 file pointer to raw data (000695A3 to 000695A6)
>   695A7 file pointer to relocation table
>       0 file pointer to line numbers
>       1 number of relocations
>       0 number of line numbers
> 40300040 flags
>         Initialized Data
>         4 byte align
>         Read Only
>
> RAW DATA #4F1
>  00000000: 00 00 00 00                                      ....
>
> RELOCATIONS #4F1
>                                                Symbol    Symbol
>  Offset    Type              Applied To         Index     Name
>  --------  ----------------  -----------------  --------  ------
>  00000000  DIR32                      00000000       D86
> ??__Eg_hosts_map@?A0xc934e6d3@appcache@@YAXXZ (void __cdecl
> appcache::`anonymous
> namespace'::`dynamic initializer for 'g_hosts_map''(void))
>
> D86 00000000 SECT4E2 notype ()    Static      |
> ??__Eg_hosts_map@?A0xc934e6d3@appcache@@YAXXZ (void __cdecl
> appcache::`anonymous
> namespace'::`dynamic initializer for 'g_hosts_map''(void))
>
>  00000A73  SECREL                     00000000       DA2  _g_hosts_map
>  00000A77  SECTION                        0000       DA2  _g_hosts_map
>  00000AB4  SECREL                     00000000       DAB
> _g_hosts_map$initializer$
>  00000AB8  SECTION                        0000       DAB
> _g_hosts_map$initializer$
>
> ... and a bunch of other junk
>
> http://codereview.chromium.org/7756015/
>

[michaeln, 2011-09-01 19:53:40]

After seeing the extra bloat introduced to the binary because this is wrapped up
in a LazyInstance, i've got another motivation to get rid of it aside from any
(or not) code execution at static init time... there's no need for that binary
baggage.

[michaeln, 2011-09-06 19:12:26]

ping

[Evan Martin, 2011-09-06 19:14:19]

LGTM

[Evan Martin, 2011-09-06 19:14:22]

LGTM