Thank you for review, Dan.

Tao

On 7/2/13 1:32 PM, Daniel D. Daugherty wrote:
Adding hotspot-runtime-...@openjdk.java.net to this thread...

> http://cr.openjdk.java.net/~tamao/7122222_hsx24/webrev.01/

make/linux/makefiles/vm.make
    No comments.

make/solaris/makefiles/vm.make
    No comments.

src/os/solaris/vm/os_solaris.inline.hpp
    No comments.

Also compared the two patch files:
    7122222GCLogLimitedTo2GFor32Bit_hsx24.patch
    7122222GCLogLimitedTo2GFor32Bit.patch

In HSX-25, the Makefile construct looks like:

CXXFLAGS/ostream.o += -D_FILE_OFFSET_BITS=64

In HSX-24, the Makefile construct looks like:

ostream.o: CXXFLAGS += -D_FILE_OFFSET_BITS=64

which threw me for a loop for a minute... I realized that
your construct matches the existing rules for vm_version.o
in each release so I think the HSX-24 backport is good.
I should've mentioned the point that I just would like to keep consistency with the existing way of passing cxxflag to vm_version.o in order to make the changeset simple to come up.

Hopefully, you double checked builds logs on both Linux
and Solaris and made sure that '-D_FILE_OFFSET_BITS=64'
is only passed where you expect it.
Yes, it is only passed "-D_FILE_OFFSET_BITS=64" to ostream.o

As I mentioned, the changeset passed JPRT.

Also, I manually tested whether the change would solve the CR's problem as expected. It's doing its right job on Linux-i586, Solaris-i586 and Solaris-sparc.

Thumbs up!

Dan


-------- Original Message --------
Subject: Re: hsx24 backport: Request for review: 7122222: GC log is limited to 2G for 32-bit
Date:   Mon, 01 Jul 2013 23:05:46 -0700
From:   Tao Mao <tao....@oracle.com>
Organization:   Oracle Corporation
To: hotspot-gc-...@openjdk.java.net <hotspot-gc-...@openjdk.java.net>, build-dev@openjdk.java.net



The format of passing cxxflag to a specific file is different in hsx24
than in hsx25. I've changed the setting accordingly in order to pass
through the compilation.

Please review the new webrev below and ignore webrev.00.

hsx24 webrev:
http://cr.openjdk.java.net/~tamao/7122222_hsx24/webrev.01/

(original) hsx25 webrev:
http://cr.openjdk.java.net/~tamao/7122222/webrev.00/

Test:
Builds were tested on Linux-i586, Solaris-i586, and Solaris-sparc.
Builds were successful and they all passed test of the gc-log size limit
of 2G.

Thanks.
Tao

On 7/1/13 1:18 PM, Tao Mao wrote:
>  The hsx25 fix has been pushed already. Then I got the
>  7u40-critical-approved.
>
>  When I tried backporting to hsx24, it didn't apply to the hsx24 repo
>  cleanly. Thus, I made the patch manually (copy-and-paste style) and
>  now need some quick reviews to get it in since it's 7u40 related P3 CR.
>
>  hsx24 webrev:
>  http://cr.openjdk.java.net/~tamao/7122222_hsx24/webrev.00/
>
>  (original) hsx25 webrev:
>  http://cr.openjdk.java.net/~tamao/7122222/webrev.00/
>
>  Thanks.
>  Tao



Reply via email to