Avoid fragmenting address space on small windows systems.

Prevent PageAllocator from fragmenting address space on small Windows systems.

Peform a runtime check in the 32-bit windows code only to see if the
address space is highly constrained by the OS. If so, we give up some
randomness in exchange for memory utilization efficiency.

BUG=394591
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178586

[Tom Sepez, 2014-07-18 17:46:57]

Chris, please take a look.  Thanks.

[Chris Evans, 2014-07-18 18:03:47]

https://codereview.chromium.org/408483004/diff/1/Source/wtf/PageAllocator.cpp
File Source/wtf/PageAllocator.cpp (right):

https://codereview.chromium.org/408483004/diff/1/Source/wtf/PageAllocator.cpp...
Source/wtf/PageAllocator.cpp:71: if (systemInfo.wProcessorArchitecture ==
PROCESSOR_ARCHITECTURE_INTEL)
I don't understand this. It sounds from the CL description that you're trying to
distinguish between a 2G/2G split vs. 3G/1G user/kernel split? If this above
code is the magic to do that, it might use a comment.

[Tom Sepez, 2014-07-18 18:10:06]

https://codereview.chromium.org/408483004/diff/1/Source/wtf/PageAllocator.cpp
File Source/wtf/PageAllocator.cpp (right):

https://codereview.chromium.org/408483004/diff/1/Source/wtf/PageAllocator.cpp...
Source/wtf/PageAllocator.cpp:71: if (systemInfo.wProcessorArchitecture ==
PROCESSOR_ARCHITECTURE_INTEL)
On 2014/07/18 18:03:47, Chris Evans wrote:
> I don't understand this. It sounds from the CL description that you're trying
to
> distinguish between a 2G/2G split vs. 3G/1G user/kernel split? If this above
> code is the magic to do that, it might use a comment.

Exactly.  Will add comment.

[Chris Evans, 2014-07-18 18:21:00]

https://codereview.chromium.org/408483004/diff/10002/Source/wtf/PageAllocator...
File Source/wtf/PageAllocator.cpp (right):

https://codereview.chromium.org/408483004/diff/10002/Source/wtf/PageAllocator...
Source/wtf/PageAllocator.cpp:72: if (systemInfo.wProcessorArchitecture ==
PROCESSOR_ARCHITECTURE_INTEL)
Ah. Maybe I get it now. Is the 3G/1G for Windows 32-bit processes restricted to
when the underlying CPU is x86_64? I still find it a bit unclear.

Would it be clearer to use the lpMaximumApplicationAddress field of this
structure? If it is over 0x80000000 then we can use the hints, otherwise not?

[Tom Sepez, 2014-07-18 18:22:50]

On 2014/07/18 18:21:00, Chris Evans wrote:
>
https://codereview.chromium.org/408483004/diff/10002/Source/wtf/PageAllocator...
> File Source/wtf/PageAllocator.cpp (right):
> 
>
https://codereview.chromium.org/408483004/diff/10002/Source/wtf/PageAllocator...
> Source/wtf/PageAllocator.cpp:72: if (systemInfo.wProcessorArchitecture ==
> PROCESSOR_ARCHITECTURE_INTEL)
> Ah. Maybe I get it now. Is the 3G/1G for Windows 32-bit processes restricted
to
> when the underlying CPU is x86_64? I still find it a bit unclear.
> 
Yep.

> Would it be clearer to use the lpMaximumApplicationAddress field of this
> structure? If it is over 0x80000000 then we can use the hints, otherwise not?
Maybe.  Justin?

[Chris Evans, 2014-07-18 18:32:35]

On 2014/07/18 18:22:50, Tom Sepez wrote:
> On 2014/07/18 18:21:00, Chris Evans wrote:
> >
>
https://codereview.chromium.org/408483004/diff/10002/Source/wtf/PageAllocator...
> > File Source/wtf/PageAllocator.cpp (right):
> > 
> >
>
https://codereview.chromium.org/408483004/diff/10002/Source/wtf/PageAllocator...
> > Source/wtf/PageAllocator.cpp:72: if (systemInfo.wProcessorArchitecture ==
> > PROCESSOR_ARCHITECTURE_INTEL)
> > Ah. Maybe I get it now. Is the 3G/1G for Windows 32-bit processes restricted
> to
> > when the underlying CPU is x86_64? I still find it a bit unclear.
> > 
> Yep.

And we get the right result for x64 CPU with a 32-bit Windows installation?

> 
> > Would it be clearer to use the lpMaximumApplicationAddress field of this
> > structure? If it is over 0x80000000 then we can use the hints, otherwise
not?
> Maybe.  Justin?

Alternatively, just comment it a bit more so that Windows dumbasses like me can
get it right away. Up to you.

[Tom Sepez, 2014-07-21 16:38:28]

Are you OK with this comment, Chris?

[jschuh, 2014-07-21 16:43:45]

https://codereview.chromium.org/408483004/diff/60001/Source/wtf/PageAllocator...
File Source/wtf/PageAllocator.cpp (right):

https://codereview.chromium.org/408483004/diff/60001/Source/wtf/PageAllocator...
Source/wtf/PageAllocator.cpp:69: // is running under WOW64, then it has 3 GB
available, and we want use the
nit: Actually, it usually has 4GB available. I think Vista and XP 64 are the
only ones that caps at 3GB.

Outside of that, I approve of this message. :)

[Chris Evans, 2014-07-21 18:02:53]

LGTM. Much clearer now, thanks.

One final suggestion; take it or leave it at your discretion!

https://codereview.chromium.org/408483004/diff/80001/Source/wtf/PageAllocator...
File Source/wtf/PageAllocator.cpp (right):

https://codereview.chromium.org/408483004/diff/80001/Source/wtf/PageAllocator...
Source/wtf/PageAllocator.cpp:65: #if !CPU(X86_64)
Nit: probably clearer with the new-ish:
#if CPU(32BIT)

[Tom Sepez, 2014-07-21 20:07:33]

Source/wtf/PageAllocator.cpp:65: #if !CPU(X86_64)
> Nit: probably clearer with the new-ish:
> #if CPU(32BIT)
Ooooh.  Shiny.  I like how 32BIT in and of itself isn't a legal identifier yet
this is all cool through the magic of token pasting.

[Tom Sepez, 2014-07-21 20:09:13]

The CQ bit was checked by tsepez@chromium.org

[commit-bot: I haz the power, 2014-07-21 20:09:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tsepez@chromium.org/408483004/120001

[commit-bot: I haz the power, 2014-07-21 21:10:36]

Change committed as 178586

[Tom Sepez, 2014-11-12 17:56:05]

On 2014/07/21 21:10:36, I haz the power (commit-bot) wrote:
> Change committed as 178586
Planning to revert due to compatibility of IsWow64Process().

[Tom Sepez, 2014-11-12 18:02:29]

On 2014/11/12 17:56:05, Tom Sepez wrote:
> On 2014/07/21 21:10:36, I haz the power (commit-bot) wrote:
> > Change committed as 178586
> Planning to revert due to compatibility of IsWow64Process().
Revert CL is https://codereview.chromium.org/721813002/