Add v8_os_page_size flag for cross compilation

Add v8_os_page_size flag for cross compilation

When generating snapshot on a machine with a different page size than
the target machine, we can run into problems as the v8 page area size
changes. This is because v8 has page guards which depend on os page
size, so if the target has larger os page, v8 page area is smaller and
may not fit the contents.

The solution proposed here is adding a flag, v8_os_page_size, that
would, if used, override local os page size and use the one specified
during snapshot generation.

BUG=

Committed: https://crrev.com/a18be72c8e7ecc3e6b0803c2287a8bbf4640a754
Cr-Commit-Position: refs/heads/master@{#40997}

[miran.karic, 2016-11-11 15:09:15]

miran.karic@imgtec.com changed reviewers:
+ hablich@chromium.org, hpayer@chromium.org, ivica.bogosavljevic@imgtec.com,
mlippautz@chromium.org, ulan@chromium.org

[miran.karic, 2016-11-11 15:09:16]

PTAL. Feel free to suggest changes in flag name and description as well as in
ways to accomplish this.

[Michael Lippautz, 2016-11-14 09:24:43]

mlippautz@chromium.org changed reviewers:
+ machenbach@chromium.org

[Michael Lippautz, 2016-11-14 09:24:44]

+machenbach

https://codereview.chromium.org/2488403003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2488403003/diff/1/BUILD.gn#newcode69
BUILD.gn:69: target_os_page_size = "0"
I'd make this v8_target_os_page_size. Not sure we need "target" in the name, as
we basically just override the os page size.

https://codereview.chromium.org/2488403003/diff/1/src/deoptimizer.cc
File src/deoptimizer.cc (right):

https://codereview.chromium.org/2488403003/diff/1/src/deoptimizer.cc#newcode27
src/deoptimizer.cc:27: FLAG_target_os_page_size != 0
The deoptimizer already uses the allocator here, so we might as well use
MemoryAllocator::GetCommitPageSize(). (See below)

https://codereview.chromium.org/2488403003/diff/1/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2488403003/diff/1/src/heap/spaces.cc#newcode23
src/heap/spaces.cc:23: intptr_t GetCommitPageSize() {
For consistency, this should go as a static function onto MemoryAllocator.

[Michael Achenbach, 2016-11-14 09:59:02]

https://codereview.chromium.org/2488403003/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2488403003/diff/1/BUILD.gn#newcode69
BUILD.gn:69: target_os_page_size = "0"
On 2016/11/14 09:24:43, Michael Lippautz wrote:
> I'd make this v8_target_os_page_size. Not sure we need "target" in the name,
as
> we basically just override the os page size.

Yes, please add a v8_ prefix. I don't care about the rest of the name, but we'd
like to know which args are from V8 in a chromium context.

[miran.karic, 2016-11-14 15:39:59]

Done. Also fixed a typo and moved v8_os_page_size flag in flag-definitions.h to
flags in all modes (was in debug only).

[miran.karic, 2016-11-14 15:40:36]

Description was changed from

==========
Add target_os_page_size flag for cross compilation

When generating snapshot on a machine with a different page size than
the target machine, we can run into problems as the v8 page area size
changes. This is because v8 has page guards which depend on os page
size, so if the target has larger os page, v8 page area is smaller and
may not fit the contents.

The solution proposed here is adding a flag, target_os_page_size, that
would, if used, override local os page size and use the one specified
during snapshot generation.

BUG=
==========

to

==========
Add v8_os_page_size flag for cross compilation

When generating snapshot on a machine with a different page size than
the target machine, we can run into problems as the v8 page area size
changes. This is because v8 has page guards which depend on os page
size, so if the target has larger os page, v8 page area is smaller and
may not fit the contents.

The solution proposed here is adding a flag, v8_os_page_size, that
would, if used, override local os page size and use the one specified
during snapshot generation.

BUG=
==========

[Michael Lippautz, 2016-11-14 16:04:08]

lgtm % comments

https://codereview.chromium.org/2488403003/diff/20001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/2488403003/diff/20001/src/flag-definitions.h#...
src/flag-definitions.h:771: DEFINE_INT(v8_os_page_size, 0, "override os page
size when generating snapshot")
"override OS page size (in KBytes)"

I think kbytes makes sense, leave it in bytes if you disagree.

https://codereview.chromium.org/2488403003/diff/20001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2488403003/diff/20001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:1024: if (FLAG_v8_os_page_size != 0)
DCHECK(IsPowerOfTwo32...)

https://codereview.chromium.org/2488403003/diff/20001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:1025: return FLAG_v8_os_page_size;
FLAG_v8_os_page_size * KB;

[miran.karic, 2016-11-14 16:40:30]

On 2016/11/14 16:04:08, Michael Lippautz wrote:
> lgtm % comments
> 
> https://codereview.chromium.org/2488403003/diff/20001/src/flag-definitions.h
> File src/flag-definitions.h (right):
> 
>
https://codereview.chromium.org/2488403003/diff/20001/src/flag-definitions.h#...
> src/flag-definitions.h:771: DEFINE_INT(v8_os_page_size, 0, "override os page
> size when generating snapshot")
> "override OS page size (in KBytes)"
> 
> I think kbytes makes sense, leave it in bytes if you disagree.
> 
> https://codereview.chromium.org/2488403003/diff/20001/src/heap/spaces.cc
> File src/heap/spaces.cc (right):
> 
>
https://codereview.chromium.org/2488403003/diff/20001/src/heap/spaces.cc#newc...
> src/heap/spaces.cc:1024: if (FLAG_v8_os_page_size != 0)
> DCHECK(IsPowerOfTwo32...)
> 
>
https://codereview.chromium.org/2488403003/diff/20001/src/heap/spaces.cc#newc...
> src/heap/spaces.cc:1025: return FLAG_v8_os_page_size;
> FLAG_v8_os_page_size * KB;

Makes sense, absolutely, done.

[miran.karic, 2016-11-15 13:08:52]

The CQ bit was checked by miran.karic@imgtec.com

[miran.karic, 2016-11-15 13:08:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlippautz@chromium.org
Link to the patchset: https://codereview.chromium.org/2488403003/#ps40001
(title: "Flag in KBytes, add DCHECK")

[commit-bot: I haz the power, 2016-11-15 13:08:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-15 13:10:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-15 13:10:10]

Try jobs failed on following builders:
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)

[miran.karic, 2016-11-15 14:08:50]

The CQ bit was unchecked by miran.karic@imgtec.com

[miran.karic, 2016-11-15 14:12:28]

The CQ bit was checked by miran.karic@imgtec.com

[miran.karic, 2016-11-15 14:12:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlippautz@chromium.org
Link to the patchset: https://codereview.chromium.org/2488403003/#ps60001
(title: "merge with master")

[commit-bot: I haz the power, 2016-11-15 14:12:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-15 14:40:07]

Description was changed from

==========
Add v8_os_page_size flag for cross compilation

When generating snapshot on a machine with a different page size than
the target machine, we can run into problems as the v8 page area size
changes. This is because v8 has page guards which depend on os page
size, so if the target has larger os page, v8 page area is smaller and
may not fit the contents.

The solution proposed here is adding a flag, v8_os_page_size, that
would, if used, override local os page size and use the one specified
during snapshot generation.

BUG=
==========

to

==========
Add v8_os_page_size flag for cross compilation

When generating snapshot on a machine with a different page size than
the target machine, we can run into problems as the v8 page area size
changes. This is because v8 has page guards which depend on os page
size, so if the target has larger os page, v8 page area is smaller and
may not fit the contents.

The solution proposed here is adding a flag, v8_os_page_size, that
would, if used, override local os page size and use the one specified
during snapshot generation.

BUG=
==========

[commit-bot: I haz the power, 2016-11-15 14:40:09]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-17 22:34:18]

Description was changed from

==========
Add v8_os_page_size flag for cross compilation

When generating snapshot on a machine with a different page size than
the target machine, we can run into problems as the v8 page area size
changes. This is because v8 has page guards which depend on os page
size, so if the target has larger os page, v8 page area is smaller and
may not fit the contents.

The solution proposed here is adding a flag, v8_os_page_size, that
would, if used, override local os page size and use the one specified
during snapshot generation.

BUG=
==========

to

==========
Add v8_os_page_size flag for cross compilation

When generating snapshot on a machine with a different page size than
the target machine, we can run into problems as the v8 page area size
changes. This is because v8 has page guards which depend on os page
size, so if the target has larger os page, v8 page area is smaller and
may not fit the contents.

The solution proposed here is adding a flag, v8_os_page_size, that
would, if used, override local os page size and use the one specified
during snapshot generation.

BUG=

Committed: https://crrev.com/a18be72c8e7ecc3e6b0803c2287a8bbf4640a754
Cr-Commit-Position: refs/heads/master@{#40997}
==========

[commit-bot: I haz the power, 2016-11-17 22:34:19]

Patchset 4 (id:??) landed as
https://crrev.com/a18be72c8e7ecc3e6b0803c2287a8bbf4640a754
Cr-Commit-Position: refs/heads/master@{#40997}