Re-land: 178586 Avoid fragmenting address space on small windows systems.

Re-land: 178586 Avoid fragmenting address space on small windows systems.

The concern about IsWow64Process() is not founded per jschuh@ (but the effect of
changing the allocator behaviour may be). We now have a clean baseline to test
against.

R=cevans@chromium.org
BUG=394591
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185559

[Tom Sepez, 2014-11-12 23:55:48]

Chris, now that we have a clean baseline, lets see what the bots say.

[Chris Evans, 2014-11-14 01:48:16]

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

https://codereview.chromium.org/717263005/diff/1/Source/wtf/PageAllocator.cpp...
Source/wtf/PageAllocator.cpp:65: #if !CPU(64BIT)
I kind of like having the symmetry of both CPU(32BIT) and CPU(64BIT) to be
honest. My vote is to keep CPU(32BIT).

[Tom Sepez, 2014-11-18 18:25:20]

> I kind of like having the symmetry of both CPU(32BIT) and CPU(64BIT) to be
> honest. My vote is to keep CPU(32BIT).
Kept 32BIT.  Thay way, we'll be safe when 128bit CPUs hit :).

[Chris Evans, 2014-11-18 18:29:32]

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

https://codereview.chromium.org/717263005/diff/20001/Source/wtf/PageAllocator...
Source/wtf/PageAllocator.cpp:74: IsWow64Process(GetCurrentProcess(), &bIsWow64);
This function has a return value (success / fail). Maybe we want to explicitly
handle the "fail" case and hard wire an assumption that it's a 32-bit process in
that case?

We might also ASSERT() that it succeeds to get visibility in the debug trybot
runs that it's working ok?

[Tom Sepez, 2014-11-18 18:38:34]

> This function has a return value (success / fail). Maybe we want to explicitly
> handle the "fail" case and hard wire an assumption that it's a 32-bit process
in
> that case?
> 
> We might also ASSERT() that it succeeds to get visibility in the debug trybot
> runs that it's working ok?
Makes sense to me. Done.

[Chris Evans, 2014-11-18 18:51:56]

On 2014/11/18 18:38:34, Tom Sepez wrote:
> > This function has a return value (success / fail). Maybe we want to
explicitly
> > handle the "fail" case and hard wire an assumption that it's a 32-bit
process
> in
> > that case?
> > 
> > We might also ASSERT() that it succeeds to get visibility in the debug
trybot
> > runs that it's working ok?
> Makes sense to me. Done.

LGTM

[Tom Sepez, 2014-11-18 22:57:06]

The CQ bit was checked by tsepez@chromium.org

[commit-bot: I haz the power, 2014-11-18 22:58:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/717263005/40001

[commit-bot: I haz the power, 2014-11-19 00:29:39]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=185559

[hiroshige, 2014-12-15 13:03:11]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/806683003/ by hiroshige@chromium.org.

The reason for reverting is: This CL crashed all layout tests on XP bot..

[hiroshige, 2014-12-16 04:12:53]

Another CL https://codereview.chromium.org/740653002/ corrected CPU(32BIT)
definition, so that CPU(32BIT) is true on Windows 32-bit environment.
Before the CL 740653002, CPU(32BIT) was false and thus this CL was not enabled.
After the CL 740653002 is commited, this CL is enabled and caused crash.