code review 9805043: runtime: introduce helper persistentalloc() function

runtime: introduce helper persistentalloc() function
It is a caching wrapper around SysAlloc() that can allocate small chunks.
Use it for symtab allocations. Reduces number of symtab walks from 4 to 3
(reduces buildfuncs time from 10ms to 7.5ms on a large binary,
reduces initial heap size by 680K on the same binary).
Also can be used for type info allocation, itab allocation.
There are also several places in GC where we do the same thing,
they can be changed to use persistentalloc().
Also can be used in FixAlloc, because each instance of FixAlloc allocates
in 128K regions, which is too eager.

[dvyukov, 2013-05-27 09:33:37]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://dvyukov%40google.com@code.google.com/p/go/

[DMorsing, 2013-05-27 14:20:26]

LGTM.

https://codereview.appspot.com/9805043/diff/4001/src/pkg/runtime/malloc.goc
File src/pkg/runtime/malloc.goc (right):

https://codereview.appspot.com/9805043/diff/4001/src/pkg/runtime/malloc.goc#n...
src/pkg/runtime/malloc.goc:507: runtime·throw("persistentalloc: align is now a
power of 2");
s/now/not/

[khr1, 2013-05-27 21:35:17]

LGTM

I'd define a second constant, PersistentAllocMaxBlock, and allocate
directly above that size (instead of PersistentAllocChunk/2).  I think it
also makes sense to lower that constant a bit, to
PersistentAllocChunk/4 or PersistentAllocChunk/8
so we don't waste a lot of memory.


On Mon, May 27, 2013 at 7:20 AM, <daniel.morsing@gmail.com> wrote:

> LGTM.
>
>
> https://codereview.appspot.**com/9805043/diff/4001/src/pkg/**
>
runtime/malloc.goc<https://codereview.appspot.com/9805043/diff/4001/src/pkg/runtime/malloc.goc>
> File src/pkg/runtime/malloc.goc (right):
>
> https://codereview.appspot.**com/9805043/diff/4001/src/pkg/**
>
runtime/malloc.goc#newcode507<https://codereview.appspot.com/9805043/diff/4001/src/pkg/runtime/malloc.goc#newcode507>
> src/pkg/runtime/malloc.goc:**507: runtime·throw("**persistentalloc: align
> is
>
> now a power of 2");
> s/now/not/
>
>
https://codereview.appspot.**com/9805043/<https://codereview.appspot.com/9805...
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[dvyukov, 2013-05-28 04:01:47]

On Tue, May 28, 2013 at 1:35 AM, Keith Randall <khr@google.com> wrote:

> LGTM
>
> I'd define a second constant, PersistentAllocMaxBlock, and allocate
> directly above that size (instead of PersistentAllocChunk/2).  I think it
> also makes sense to lower that constant a bit, to PersistentAllocChunk/4
> or PersistentAllocChunk/8 so we don't waste a lot of memory.
>
>

On Windows minimum allocation granularity is 64K. If you ask for 4K, it
will commit 4K, but reserve 64K of virtual address space. It is especially
unpleasant on 32-bits.
Probably we can increase PersistentAllocChunk to 256K, then
PersistentAllocMaxBlock will be 1/4.

[dvyukov, 2013-05-28 06:47:18]

On 2013/05/28 04:01:47, dvyukov wrote:
> On Tue, May 28, 2013 at 1:35 AM, Keith Randall <mailto:khr@google.com> wrote:
> 
> > LGTM
> >
> > I'd define a second constant, PersistentAllocMaxBlock, and allocate
> > directly above that size (instead of PersistentAllocChunk/2).  I think it
> > also makes sense to lower that constant a bit, to PersistentAllocChunk/4
> > or PersistentAllocChunk/8 so we don't waste a lot of memory.
> >
> >
> 
> On Windows minimum allocation granularity is 64K. If you ask for 4K, it
> will commit 4K, but reserve 64K of virtual address space. It is especially
> unpleasant on 32-bits.
> Probably we can increase PersistentAllocChunk to 256K, then
> PersistentAllocMaxBlock will be 1/4.


Introduced PersistentAllocMaxBlock=64K constant, and bumped PersistentAllocChunk
to 256K.
I think it should be fine, because on a large binary we allocate ~500K only for
symtab.

[dvyukov, 2013-05-28 06:47:51]

*** Submitted as https://code.google.com/p/go/source/detail?r=776aba85ece8 ***

runtime: introduce helper persistentalloc() function
It is a caching wrapper around SysAlloc() that can allocate small chunks.
Use it for symtab allocations. Reduces number of symtab walks from 4 to 3
(reduces buildfuncs time from 10ms to 7.5ms on a large binary,
reduces initial heap size by 680K on the same binary).
Also can be used for type info allocation, itab allocation.
There are also several places in GC where we do the same thing,
they can be changed to use persistentalloc().
Also can be used in FixAlloc, because each instance of FixAlloc allocates
in 128K regions, which is too eager.

R=golang-dev, daniel.morsing, khr
CC=golang-dev
https://codereview.appspot.com/9805043

[dave_cheney.net, 2013-05-28 07:02:51]

On 2013/05/28 06:47:51, dvyukov wrote:
> *** Submitted as https://code.google.com/p/go/source/detail?r=776aba85ece8 ***
> 
> runtime: introduce helper persistentalloc() function
> It is a caching wrapper around SysAlloc() that can allocate small chunks.
> Use it for symtab allocations. Reduces number of symtab walks from 4 to 3
> (reduces buildfuncs time from 10ms to 7.5ms on a large binary,
> reduces initial heap size by 680K on the same binary).
> Also can be used for type info allocation, itab allocation.
> There are also several places in GC where we do the same thing,
> they can be changed to use persistentalloc().
> Also can be used in FixAlloc, because each instance of FixAlloc allocates
> in 128K regions, which is too eager.
> 
> R=golang-dev, daniel.morsing, khr
> CC=golang-dev
> https://codereview.appspot.com/9805043

This is causing build failures across all the amd64 builders. Please revert the
change.

[dvyukov, 2013-05-28 07:14:18]

weird

undo cl:
https://codereview.appspot.com/9822043



On Tue, May 28, 2013 at 11:02 AM, <dave@cheney.net> wrote:

> On 2013/05/28 06:47:51, dvyukov wrote:
>
>> *** Submitted as
>>
>
https://code.google.com/p/go/**source/detail?r=776aba85ece8<https://code.goog...
>
>  runtime: introduce helper persistentalloc() function
>> It is a caching wrapper around SysAlloc() that can allocate small
>>
> chunks.
>
>> Use it for symtab allocations. Reduces number of symtab walks from 4
>>
> to 3
>
>> (reduces buildfuncs time from 10ms to 7.5ms on a large binary,
>> reduces initial heap size by 680K on the same binary).
>> Also can be used for type info allocation, itab allocation.
>> There are also several places in GC where we do the same thing,
>> they can be changed to use persistentalloc().
>> Also can be used in FixAlloc, because each instance of FixAlloc
>>
> allocates
>
>> in 128K regions, which is too eager.
>>
>
>  R=golang-dev, daniel.morsing, khr
>> CC=golang-dev
>>
https://codereview.appspot.**com/9805043<https://codereview.appspot.com/9805043>
>>
>
> This is causing build failures across all the amd64 builders. Please
> revert the change.
>
>
https://codereview.appspot.**com/9805043/<https://codereview.appspot.com/9805...
>