CodeCacheExpansionSize too small for Zero on ppc32/ppc64

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

CodeCacheExpansionSize too small for Zero on ppc32/ppc64

John Paul Adrian Glaubitz
Hi!

During my recent tests, I noticed that Zero fails to build on both ppc32
and ppc64 with the following error message:

Optimizing the exploded image
/home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/jdk/bin/java -Xms64M -Xmx1600M -XX:ThreadStackSize=1536 -XX:+UseSerialGC -Xms32M -Xmx512M -XX:TieredStopAtLevel=1 -cp /home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/buildtools/tools_jigsaw_classes --add-exports java.base/jdk.internal.module=ALL-UNNAMED build.tools.jigsaw.AddPackagesAttribute /home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/jdk
uintx CodeCacheExpansionSize=32768 is outside the allowed range [ 65536 ... 18446744073709551615 ]
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.
ExplodedImageOptimize.gmk:40: recipe for target '/home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/jdk/_packages_attribute.done' failed
make[3]: *** [/home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/jdk/_packages_attribute.done] Error 1
make[3]: Leaving directory '/home/glaubitz/openjdk/hs/make'
make/Main.gmk:361: recipe for target 'exploded-image-optimize' failed
make[2]: *** [exploded-image-optimize] Error 2
make[2]: Leaving directory '/home/glaubitz/openjdk/hs

Looking at ./src/hotspot/share/runtime/globals.hpp, CodeCacheExpansionSize
is set to be 32768 bytes:

define_pd_global(intx, CodeCacheExpansionSize,       32*K);

Interestingly, I have not run into this issue with Zero on x86_64 or SPARC
with other architectures currently being tested. Could it be that the
value of CodeCacheExpansionSize has a minimum limit on ppc*? If yes, should
we just increase it to 64k?

Or maybe we have to find the place where it enforces the value on ppc* to
be unconditionally 64k?

Thanks,
Adrian

--
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

Tobias Hartmann-2
Hi Adrian,

seems like this is due to my fix for JDK-8179026 [1] which now enforces the CodeCacheExpansionSize to be >=
os::vm_page_size(). I did this because in CodeCache::initialize() we align the size anyway:

CodeCacheExpansionSize = align_up(CodeCacheExpansionSize, os::vm_page_size());

Since other platforms define 32*K as their default value as well, it might be best to just set the lower limit to 32*K
instead of os::vm_page_size() and let above alignment code take care of adjusting the size.

Thanks,
Tobias

[1] http://hg.openjdk.java.net/jdk/hs/rev/7f40c1cdde28

On 28.11.2017 14:36, John Paul Adrian Glaubitz wrote:

> Hi!
>
> During my recent tests, I noticed that Zero fails to build on both ppc32
> and ppc64 with the following error message:
>
> Optimizing the exploded image
> /home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/jdk/bin/java -Xms64M -Xmx1600M -XX:ThreadStackSize=1536
> -XX:+UseSerialGC -Xms32M -Xmx512M -XX:TieredStopAtLevel=1 -cp
> /home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/buildtools/tools_jigsaw_classes --add-exports
> java.base/jdk.internal.module=ALL-UNNAMED build.tools.jigsaw.AddPackagesAttribute
> /home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/jdk
> uintx CodeCacheExpansionSize=32768 is outside the allowed range [ 65536 ... 18446744073709551615 ]
> Error: Could not create the Java Virtual Machine.
> Error: A fatal exception has occurred. Program will exit.
> ExplodedImageOptimize.gmk:40: recipe for target
> '/home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/jdk/_packages_attribute.done' failed
> make[3]: *** [/home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/jdk/_packages_attribute.done] Error 1
> make[3]: Leaving directory '/home/glaubitz/openjdk/hs/make'
> make/Main.gmk:361: recipe for target 'exploded-image-optimize' failed
> make[2]: *** [exploded-image-optimize] Error 2
> make[2]: Leaving directory '/home/glaubitz/openjdk/hs
>
> Looking at ./src/hotspot/share/runtime/globals.hpp, CodeCacheExpansionSize
> is set to be 32768 bytes:
>
> define_pd_global(intx, CodeCacheExpansionSize,       32*K);
>
> Interestingly, I have not run into this issue with Zero on x86_64 or SPARC
> with other architectures currently being tested. Could it be that the
> value of CodeCacheExpansionSize has a minimum limit on ppc*? If yes, should
> we just increase it to 64k?
>
> Or maybe we have to find the place where it enforces the value on ppc* to
> be unconditionally 64k?
>
> Thanks,
> Adrian
>
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

John Paul Adrian Glaubitz
Hi Tobias!

On 11/28/2017 03:15 PM, Tobias Hartmann wrote:
> seems like this is due to my fix for JDK-8179026 [1] which now enforces the CodeCacheExpansionSize to be >=
> os::vm_page_size(). I did this because in CodeCache::initialize() we align the size anyway:
>
> CodeCacheExpansionSize = align_up(CodeCacheExpansionSize, os::vm_page_size());
>
> Since other platforms define 32*K as their default value as well, it might be best to just set the lower limit to 32*K
> instead of os::vm_page_size() and let above alignment code take care of adjusting the size.
Thanks a lot for your quick reply! Good to know that this regression was just
introduced recently, I already thought I missed some other ppc-specific patch.

So, looking at your changeset:

> http://hg.openjdk.java.net/jdk/hs/rev/7f40c1cdde28#l3.1

Should we just set the sizes there from the fixed values defined in globals.hpp
for all the cases where it's set to os::vm_page_size() or just for
CodeCacheMinBlockLength?

Adrian

--
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

Tobias Hartmann-2
Hi Adrian,

On 28.11.2017 15:21, John Paul Adrian Glaubitz wrote:
> Should we just set the sizes there from the fixed values defined in globals.hpp
> for all the cases where it's set to os::vm_page_size() or just for
> CodeCacheMinBlockLength?

I think we should just fix it for CodeCacheExpansionSize

-          range(os::vm_page_size(), max_uintx)                              \
+          range(32*K, max_uintx)                                            \

The default values for the other flags are much larger than vm_page_size() and definitely should be.

I can take care of fixing this - since I also broke it :)

Thanks for reporting!

Best regards,
Tobias
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

John Paul Adrian Glaubitz
On 11/28/2017 03:31 PM, Tobias Hartmann wrote:
> I think we should just fix it for CodeCacheExpansionSize
>
> -          range(os::vm_page_size(), max_uintx)                              \
> +          range(32*K, max_uintx)                                            \
>
> The default values for the other flags are much larger than vm_page_size() and definitely should be.

Alternatively, wouldn't it make more sense to set CodeCacheExpansionSize
in globals.hpp to os::vm_page_size()? I mean, then your change would
still be in and we still would fix the problem.

> I can take care of fixing this - since I also broke it :)

Ok, let me test the change first real quick just to be safe.

> Thanks for reporting!

Thanks for being so quick fixing it!

Adrian

--
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

Tobias Hartmann-2

> Alternatively, wouldn't it make more sense to set CodeCacheExpansionSize
> in globals.hpp to os::vm_page_size()? I mean, then your change would
> still be in and we still would fix the problem.

Yes, but to be on the safe side, we should fix the 32*K defaults on all platforms:
http://cr.openjdk.java.net/~thartmann/8191996/webrev.01/

> Ok, let me test the change first real quick just to be safe.

Thanks! Please let me know if there are still problems.

> Thanks for being so quick fixing it!

Sure, I've filed JDK-8191996 and will send the fix for review soon.

Best regards,
Tobias
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

John Paul Adrian Glaubitz
Hi Tobias!

On 11/28/2017 04:01 PM, Tobias Hartmann wrote:

>
>> Alternatively, wouldn't it make more sense to set CodeCacheExpansionSize
>> in globals.hpp to os::vm_page_size()? I mean, then your change would
>> still be in and we still would fix the problem.
>
> Yes, but to be on the safe side, we should fix the 32*K defaults on all platforms:
> http://cr.openjdk.java.net/~thartmann/8191996/webrev.01/
>
>> Ok, let me test the change first real quick just to be safe.
>
> Thanks! Please let me know if there are still problems.

Will test the change on all relevant targets now. Might take a bit
longer, but I'll report back as soon as possible.

>> Thanks for being so quick fixing it!
>
> Sure, I've filed JDK-8191996 and will send the fix for review soon.

Ok, thanks.

Adrian

--
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

John Paul Adrian Glaubitz
On 11/28/2017 04:07 PM, John Paul Adrian Glaubitz wrote:
> Will test the change on all relevant targets now. Might take a bit
> longer, but I'll report back as soon as possible.

Looks like you're missing an include here:

if test `/usr/bin/wc -l < /home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/make-support/failure-logs/hotspot_variant-zero_libjvm_objs_basicLock.o.log` -gt 12; then /bin/echo "   ... (rest of output omitted)" ; fi
/usr/bin/printf "* For target hotspot_variant-zero_libjvm_objs_biasedLocking.o:\n"
* For target hotspot_variant-zero_libjvm_objs_biasedLocking.o:
(/bin/grep -v -e "^Note: including file:" <  /home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/make-support/failure-logs/hotspot_variant-zero_libjvm_objs_biasedLocking.o.log || true) | /usr/bin/head -n 12
In file included from /home/glaubitz/openjdk/hs/src/hotspot/share/utilities/debug.hpp:30:0,
                  from /home/glaubitz/openjdk/hs/src/hotspot/share/utilities/globalDefinitions.hpp:29,
                  from /home/glaubitz/openjdk/hs/src/hotspot/share/utilities/align.hpp:28,
                  from /home/glaubitz/openjdk/hs/src/hotspot/share/runtime/globals.hpp:28,
                  from /home/glaubitz/openjdk/hs/src/hotspot/share/memory/allocation.hpp:28,
                  from /home/glaubitz/openjdk/hs/src/hotspot/share/logging/logLevel.hpp:27,
                  from /home/glaubitz/openjdk/hs/src/hotspot/share/logging/log.hpp:27,
                  from /home/glaubitz/openjdk/hs/src/hotspot/share/runtime/biasedLocking.cpp:26:
/home/glaubitz/openjdk/hs/src/hotspot/share/runtime/globals.hpp:93:54: error: ‘os’ has not been declared
  define_pd_global(intx, CodeCacheExpansionSize,       os::vm_page_size());

--
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

Tobias Hartmann-2

On 28.11.2017 16:12, John Paul Adrian Glaubitz wrote:

> Looks like you're missing an include here:
>
> if test `/usr/bin/wc -l <
> /home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/make-support/failure-logs/hotspot_variant-zero_libjvm_objs_basicLock.o.log`
> -gt 12; then /bin/echo "   ... (rest of output omitted)" ; fi
> /usr/bin/printf "* For target hotspot_variant-zero_libjvm_objs_biasedLocking.o:\n"
> * For target hotspot_variant-zero_libjvm_objs_biasedLocking.o:
> (/bin/grep -v -e "^Note: including file:" < 
> /home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/make-support/failure-logs/hotspot_variant-zero_libjvm_objs_biasedLocking.o.log
> || true) | /usr/bin/head -n 12
> In file included from /home/glaubitz/openjdk/hs/src/hotspot/share/utilities/debug.hpp:30:0,
>                  from /home/glaubitz/openjdk/hs/src/hotspot/share/utilities/globalDefinitions.hpp:29,
>                  from /home/glaubitz/openjdk/hs/src/hotspot/share/utilities/align.hpp:28,
>                  from /home/glaubitz/openjdk/hs/src/hotspot/share/runtime/globals.hpp:28,
>                  from /home/glaubitz/openjdk/hs/src/hotspot/share/memory/allocation.hpp:28,
>                  from /home/glaubitz/openjdk/hs/src/hotspot/share/logging/logLevel.hpp:27,
>                  from /home/glaubitz/openjdk/hs/src/hotspot/share/logging/log.hpp:27,
>                  from /home/glaubitz/openjdk/hs/src/hotspot/share/runtime/biasedLocking.cpp:26:
> /home/glaubitz/openjdk/hs/src/hotspot/share/runtime/globals.hpp:93:54: error: ‘os’ has not been declared
>  define_pd_global(intx, CodeCacheExpansionSize,       os::vm_page_size());

Thanks, I've just noticed this as well but it seems we cannot include runtime/os.hpp from globals.hpp without
introducing circular dependencies..

I guess it's best to go with the straight forward fix of setting the min value to 32*K:
http://cr.openjdk.java.net/~thartmann/8191996/webrev.00/

What do you think?

Best regards,
Tobias
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

John Paul Adrian Glaubitz
On 11/28/2017 04:36 PM, Tobias Hartmann wrote:
> Thanks, I've just noticed this as well but it seems we cannot include runtime/os.hpp from globals.hpp without
> introducing circular dependencies..

Hmm, we might be able to reorder some include directions. I will have a look
at this.

> I guess it's best to go with the straight forward fix of setting the min value to 32*K:
> http://cr.openjdk.java.net/~thartmann/8191996/webrev.00/

I guess that works, too, but I'm not too happy with it unless we have made sure
there isn't really any other option.

Adrian

--
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

Tobias Hartmann-2

On 28.11.2017 16:43, John Paul Adrian Glaubitz wrote:
> Hmm, we might be able to reorder some include directions. I will have a look
> at this.

I've tried this but I don't think it's easily possible.

> I guess that works, too, but I'm not too happy with it unless we have made sure
> there isn't really any other option.

Right, I agree but we still need to fix this quickly because it blocks integration. I've sent a RFR:
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-November/027723.html

If it turns out that there is a better way, we can still push a follow-up fix. With respect to behavior there is no
difference because the default size gets aligned to page-size anyway.

Thanks,
Tobias
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

David Holmes
In reply to this post by John Paul Adrian Glaubitz
 >   define_pd_global(intx, CodeCacheExpansionSize,
os::vm_page_size());

I thought the values here had to be constants ?? The appropriate level
of initialization may not have happened when this default value would be
assigned!

David

On 29/11/2017 1:12 AM, John Paul Adrian Glaubitz wrote:

> On 11/28/2017 04:07 PM, John Paul Adrian Glaubitz wrote:
>> Will test the change on all relevant targets now. Might take a bit
>> longer, but I'll report back as soon as possible.
>
> Looks like you're missing an include here:
>
> if test `/usr/bin/wc -l <
> /home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/make-support/failure-logs/hotspot_variant-zero_libjvm_objs_basicLock.o.log`
> -gt 12; then /bin/echo "   ... (rest of output omitted)" ; fi
> /usr/bin/printf "* For target
> hotspot_variant-zero_libjvm_objs_biasedLocking.o:\n"
> * For target hotspot_variant-zero_libjvm_objs_biasedLocking.o:
> (/bin/grep -v -e "^Note: including file:" <  
> /home/glaubitz/openjdk/hs/build/linux-ppc64-normal-zero-release/make-support/failure-logs/hotspot_variant-zero_libjvm_objs_biasedLocking.o.log
> || true) | /usr/bin/head -n 12
> In file included from
> /home/glaubitz/openjdk/hs/src/hotspot/share/utilities/debug.hpp:30:0,
>                   from
> /home/glaubitz/openjdk/hs/src/hotspot/share/utilities/globalDefinitions.hpp:29,
>
>                   from
> /home/glaubitz/openjdk/hs/src/hotspot/share/utilities/align.hpp:28,
>                   from
> /home/glaubitz/openjdk/hs/src/hotspot/share/runtime/globals.hpp:28,
>                   from
> /home/glaubitz/openjdk/hs/src/hotspot/share/memory/allocation.hpp:28,
>                   from
> /home/glaubitz/openjdk/hs/src/hotspot/share/logging/logLevel.hpp:27,
>                   from
> /home/glaubitz/openjdk/hs/src/hotspot/share/logging/log.hpp:27,
>                   from
> /home/glaubitz/openjdk/hs/src/hotspot/share/runtime/biasedLocking.cpp:26:
> /home/glaubitz/openjdk/hs/src/hotspot/share/runtime/globals.hpp:93:54:
> error: ‘os’ has not been declared
>   define_pd_global(intx, CodeCacheExpansionSize,       os::vm_page_size());
>
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

John Paul Adrian Glaubitz
On 11/28/2017 10:11 PM, David Holmes wrote:
>>   define_pd_global(intx, CodeCacheExpansionSize, os::vm_page_size());
>
> I thought the values here had to be constants ?? The appropriate level of initialization may not have happened when this default value would be assigned!

Does this apply to all the initialization values set here?

> http://hg.openjdk.java.net/jdk/hs/rev/7f40c1cdde28#l3.1

Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - [hidden email]
`. `'   Freie Universitaet Berlin - [hidden email]
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

David Holmes
On 29/11/2017 7:39 AM, John Paul Adrian Glaubitz wrote:
> On 11/28/2017 10:11 PM, David Holmes wrote:
>>>     define_pd_global(intx, CodeCacheExpansionSize, os::vm_page_size());
>>
>> I thought the values here had to be constants ?? The appropriate level of initialization may not have happened when this default value would be assigned!
>
> Does this apply to all the initialization values set here?
>
>> http://hg.openjdk.java.net/jdk/hs/rev/7f40c1cdde28#l3.1

I'd certainly want to investigate exactly when in the initialization
sequence these will get evaluated. I think range usage may be safer than
default value, but this needs examining!

David
-----

> Adrian
>
Reply | Threaded
Open this post in threaded view
|

Re: CodeCacheExpansionSize too small for Zero on ppc32/ppc64

Tobias Hartmann-2
Hi,

On 29.11.2017 00:34, David Holmes wrote:
>>> I thought the values here had to be constants ?? The appropriate level of initialization may not have happened when
>>> this default value would be assigned!

Yes, I came to the same conclusion. I've fixed it by using a constant lower bound:
http://hg.openjdk.java.net/jdk/hs/rev/646ed97b7e0d

> I'd certainly want to investigate exactly when in the initialization sequence these will get evaluated. I think range
> usage may be safer than default value, but this needs examining!

Yes, I think the range usage is okay because it's only evaluated during argument processing
(Arguments::process_argument()). We have existing uses in range checks:
http://hg.openjdk.java.net/jdk/hs/file/5a449dbca6d0/src/hotspot/share/runtime/globals.hpp#l643

Thanks,
Tobias