Contribution of PowerPC port (continuation of 422063005) - PPC opt 2

Contribution of PowerPC port (continuation of 422063005) - PPC opt 2

Contribution of PowerPC port (continuation of 422063005, 817143002,
866843003, and 901083004. The bulk of the changes are to remove some
hard coded assumptions about heap page size within existing tests.
The remaining change is to use a larger heap page size for PPC linux
as this provides a performance benefit due to the larger memory page size.

	modified:   src/base/build_config.h
	modified:   src/heap/heap.cc
	modified:   test/cctest/test-alloc.cc
	modified:   test/cctest/test-constantpool.cc
	modified:   test/cctest/test-heap.cc
	modified:   test/cctest/test-spaces.cc
	modified:   test/cctest/test-weakmaps.cc
	modified:   test/cctest/test-weaksets.cc

R=danno@chromium.org, svenpanne@chromium.org

BUG=

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

[michael_dawson, 2015-02-18 22:09:38]

PPC opt 2, does not overlap with ppc directory update so we can review in
parallel

[Sven Panne, 2015-02-19 12:12:31]

svenpanne@chromium.org changed reviewers:
+ hpayer@chromium.org

[Sven Panne, 2015-02-19 12:12:31]

The changes look OK to me, but my knowledge of the GC details is limited.
@Hannes: Could you please review this?

[Hannes Payer (out of office), 2015-02-19 17:47:24]

What is the performance impact of that change? And why does it improve
performance? I guess you observe the benefit of having an initial larger
semi-space size (4M). Instead of increasing the page size, you could start with
a larger semi-space size on ppc.

[michael_dawson, 2015-02-19 18:26:49]

On 2015/02/19 17:47:24, Hannes Payer wrote:
> What is the performance impact of that change? And why does it improve
> performance? I guess you observe the benefit of having an initial larger
> semi-space size (4M). Instead of increasing the page size, you could start
with
> a larger semi-space size on ppc.

When enabled with other performance optmizations, increasing the pagesize got 
us 1-2%. Since the machine page size is larger for PPC (64k) the guard/metadata 
pages consume a larger portion of the default 1MB heap page size.

[Hannes Payer (out of office), 2015-02-19 20:32:38]

On 2015/02/19 18:26:49, michael_dawson wrote:
> On 2015/02/19 17:47:24, Hannes Payer wrote:
> > What is the performance impact of that change? And why does it improve
> > performance? I guess you observe the benefit of having an initial larger
> > semi-space size (4M). Instead of increasing the page size, you could start
> with
> > a larger semi-space size on ppc.
> 
> When enabled with other performance optmizations, increasing the pagesize got 
> us 1-2%. Since the machine page size is larger for PPC (64k) the
guard/metadata 
> pages consume a larger portion of the default 1MB heap page size.

1-2% on which benchmarks?
how does it compare to 1M pages with a --min-semi-space-size=4?

[michael_dawson, 2015-02-20 13:21:24]

On 2015/02/19 20:32:38, Hannes Payer wrote:
> On 2015/02/19 18:26:49, michael_dawson wrote:
> > On 2015/02/19 17:47:24, Hannes Payer wrote:
> > > What is the performance impact of that change? And why does it improve
> > > performance? I guess you observe the benefit of having an initial larger
> > > semi-space size (4M). Instead of increasing the page size, you could start
> > with
> > > a larger semi-space size on ppc.
> > 
> > When enabled with other performance optmizations, increasing the pagesize
got 
> > us 1-2%. Since the machine page size is larger for PPC (64k) the
> guard/metadata 
> > pages consume a larger portion of the default 1MB heap page size.
> 
> 1-2% on which benchmarks?
> how does it compare to 1M pages with a --min-semi-space-size=4?

It was 1-2% on octane.  We'll have to try out with --min-semi-space-size=4 and
compare

[michael_dawson, 2015-02-20 22:03:32]

On 2015/02/20 13:21:24, michael_dawson wrote:
> On 2015/02/19 20:32:38, Hannes Payer wrote:
> > On 2015/02/19 18:26:49, michael_dawson wrote:
> > > On 2015/02/19 17:47:24, Hannes Payer wrote:
> > > > What is the performance impact of that change? And why does it improve
> > > > performance? I guess you observe the benefit of having an initial larger
> > > > semi-space size (4M). Instead of increasing the page size, you could
start
> > > with
> > > > a larger semi-space size on ppc.
> > > 
> > > When enabled with other performance optmizations, increasing the pagesize
> got 
> > > us 1-2%. Since the machine page size is larger for PPC (64k) the
> > guard/metadata 
> > > pages consume a larger portion of the default 1MB heap page size.
> > 
> > 1-2% on which benchmarks?
> > how does it compare to 1M pages with a --min-semi-space-size=4?
> 
> It was 1-2% on octane.  We'll have to try out with --min-semi-space-size=4 and
> compare

We are still doing runs to capture the current benefit of the page size change
given changes to the OOL implementation versus the option suggested. 
Separately,
most of the changes in this patch are to make the tests use the page size define
as opposed to a hard coded assumption. Would it make sense to commit those
changes
and then submit a second review once we've captured the current benefit and
comparison to --min-semi-space-size=4 ?  If so I can update this patch to move
the change to the page size for PPC.

[Sven Panne, 2015-02-23 07:27:29]

On 2015/02/20 22:03:32, michael_dawson wrote:
> [...] Would it make sense to commit those changes
> and then submit a second review once we've captured the current benefit and
> comparison to --min-semi-space-size=4 ?  If so I can update this patch to move
> the change to the page size for PPC.

This sounds OK to me (i.e. keeping this CL without the build_config.h change).
@Hannes: Would that be OK for you, too?

[Hannes Payer (out of office), 2015-02-23 09:15:50]

Just the test files, OK.

[michael_dawson, 2015-02-23 15:06:35]

Ok, updated to remove non-test changes

[michael_dawson, 2015-02-24 17:08:13]

Any other changes needed before the changes to the test files can go in ?

[Hannes Payer (out of office), 2015-02-24 17:53:50]

lgtm

[michael_dawson, 2015-02-24 18:28:06]

The CQ bit was checked by michael_dawson@ca.ibm.com

[michael_dawson, 2015-02-24 18:28:45]

The CQ bit was unchecked by michael_dawson@ca.ibm.com

[michael_dawson, 2015-02-24 18:30:17]

The CQ bit was checked by michael_dawson@ca.ibm.com

[commit-bot: I haz the power, 2015-02-24 18:39:21]

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

[commit-bot: I haz the power, 2015-02-24 19:05:28]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-24 19:05:45]

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

[michael_dawson, 2015-02-25 21:18:54]

Related to the page size opt, We have just noticed that a recent change removed
the  kBootCodeSizeMultiplier  as a tunable and the net is that the
test-heap/FirstPageFitsStartup  fails on Power because the test assumes the
initial allocations fit into a single page.  In our case because of the larger
page size they don't.

I'm wondering how we should handle this ?

[Sven Panne, 2015-02-26 07:52:51]

On 2015/02/25 21:18:54, michael_dawson wrote:
> Related to the page size opt, We have just noticed that a recent change
removed
> the  kBootCodeSizeMultiplier  as a tunable and the net is that the
> test-heap/FirstPageFitsStartup  fails on Power because the test assumes the
> initial allocations fit into a single page.  In our case because of the larger
> page size they don't.
> 
> I'm wondering how we should handle this ?

Please contact yangguo@ regarding this issue, I don't know what the new story
here is...

[Yang, 2015-03-11 14:59:08]

On 2015/02/25 21:18:54, michael_dawson wrote:
> Related to the page size opt, We have just noticed that a recent change
removed
> the  kBootCodeSizeMultiplier  as a tunable and the net is that the
> test-heap/FirstPageFitsStartup  fails on Power because the test assumes the
> initial allocations fit into a single page.  In our case because of the larger
> page size they don't.
> 
> I'm wondering how we should handle this ?

test-heap/FirstPageFitsStartup no longer exists. Do you mean
test-spaces/SizeOfFirstPageIsLargeEnough? Does it fail due CODE_SPACE size?

[michael_dawson, 2015-03-11 19:28:50]

On 2015/03/11 14:59:08, Yang wrote:
> On 2015/02/25 21:18:54, michael_dawson wrote:
> > Related to the page size opt, We have just noticed that a recent change
> removed
> > the  kBootCodeSizeMultiplier  as a tunable and the net is that the
> > test-heap/FirstPageFitsStartup  fails on Power because the test assumes the
> > initial allocations fit into a single page.  In our case because of the
larger
> > page size they don't.
> > 
> > I'm wondering how we should handle this ?
> 
> test-heap/FirstPageFitsStartup no longer exists. Do you mean
> test-spaces/SizeOfFirstPageIsLargeEnough? Does it fail due CODE_SPACE size?

The ones we have seen fail recently that we believe are related include:

  cctest/test-strings/OneByteArrayJoin
  cctest/test-spaces/SizeOfFirstPageIsLargeEnough

We do believe they failed because of  CODE_SPACE size

[Yang, 2015-03-12 15:10:33]

On 2015/03/11 19:28:50, michael_dawson wrote:
> On 2015/03/11 14:59:08, Yang wrote:
> > On 2015/02/25 21:18:54, michael_dawson wrote:
> > > Related to the page size opt, We have just noticed that a recent change
> > removed
> > > the  kBootCodeSizeMultiplier  as a tunable and the net is that the
> > > test-heap/FirstPageFitsStartup  fails on Power because the test assumes
the
> > > initial allocations fit into a single page.  In our case because of the
> larger
> > > page size they don't.
> > > 
> > > I'm wondering how we should handle this ?
> > 
> > test-heap/FirstPageFitsStartup no longer exists. Do you mean
> > test-spaces/SizeOfFirstPageIsLargeEnough? Does it fail due CODE_SPACE size?
> 
> The ones we have seen fail recently that we believe are related include:
> 
>   cctest/test-strings/OneByteArrayJoin
>   cctest/test-spaces/SizeOfFirstPageIsLargeEnough
> 
> We do believe they failed because of  CODE_SPACE size

I can't reproduce the failures you've seen with 4ededa8694c4 in release mode.

In case of OneByteArrayJoin, it probably suffices to increase the resource
constraint. For SizeOfFirstPageIsLargeEnough I'd like to be able to reproduce it
first.

[MTBrandyberry, 2015-03-12 16:49:04]

On 2015/03/12 15:10:33, Yang wrote:
> On 2015/03/11 19:28:50, michael_dawson wrote:
> > On 2015/03/11 14:59:08, Yang wrote:
> > > On 2015/02/25 21:18:54, michael_dawson wrote:
> > > > Related to the page size opt, We have just noticed that a recent change
> > > removed
> > > > the  kBootCodeSizeMultiplier  as a tunable and the net is that the
> > > > test-heap/FirstPageFitsStartup  fails on Power because the test assumes
> the
> > > > initial allocations fit into a single page.  In our case because of the
> > larger
> > > > page size they don't.
> > > > 
> > > > I'm wondering how we should handle this ?
> > > 
> > > test-heap/FirstPageFitsStartup no longer exists. Do you mean
> > > test-spaces/SizeOfFirstPageIsLargeEnough? Does it fail due CODE_SPACE
size?
> > 
> > The ones we have seen fail recently that we believe are related include:
> > 
> >   cctest/test-strings/OneByteArrayJoin
> >   cctest/test-spaces/SizeOfFirstPageIsLargeEnough
> > 
> > We do believe they failed because of  CODE_SPACE size
> 
> I can't reproduce the failures you've seen with 4ededa8694c4 in release mode.
> 
> In case of OneByteArrayJoin, it probably suffices to increase the resource
> constraint. For SizeOfFirstPageIsLargeEnough I'd like to be able to reproduce
it
> first.

The logs from buildbot show the failures:


http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20ppc64%20-%20s...

http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20ppc64%20-%20s....

Note that this currently shows up in 64-bit only because of its excessive code
size (due to a lack of constant pool support + another optimization yet to be
discussed).