On 11/09/2020 04:48, Monica Beckwith wrote:
On Thu, 10 Sep 2020 17:09:00 GMT, Monica Beckwith <mbeck...@openjdk.org> wrote:

This looks good to me with Alexsey's changes applied.

Ref: https://github.com/openjdk/jdk/pull/97#issuecomment-690077743
Hi David,
Thanks. I wanted to start a conversation here as it does modify common code 
(the aarch64-linux combo check) and also
wanted to highlight the difference between Shenandoah and ZGC checks. IMHO, 
Shenandoah checks are cleaner and if people
agree, I would like to apply those to ZGC as well. (When that happens, it 
*will* be outside of the windows-aarch64 port
and I will change the info in JBS accordingly) Please let me know what you 
think.

Ref: https://github.com/openjdk/jdk/pull/97#issuecomment-690811308
I empathize with your comment, David. Here's a bit of a background for all -
When I started the port, both ZGC, and Shenandoah had the "xlinux-aarch64" check whereas 
the "xx86" check was more
elaborate for ZGC.  At that time I chose the ZGC route for both the GCs and 
added separate checks for each OS. This is
what the windows-aarch64 port has as well. (Since then Shenandoah has moved on 
to just checking the arch.) I already
had a JBS entry, and the (partial) patch, I decided to use that for my first 
GitHub PR. and was hoping to start the
conversation of the simpler route that Shenandoah took in the comments 
mentioning that as my preference and modify the
JBS entry (and the patch) accordingly.

Now coming back to this - @stooart-mon, do you think it is OK to just check for 
aarch64 (similar to Shenandoah)?
@stefank what do you think of simplifying the x86 checks (similar to 
Shenandoah)? Thanks all.

-------------

PR: https://git.openjdk.java.net/jdk/pull/97


Hi Monica,
I guess the choice is either to enable aarch64, and have the Mac port disable ZGC if necessary, or do what you have done here and just enable by by architecture and supported platform. If Anton is happy with the simple check for aarch64, then I'm happy.

BR,
        Stuart

Reply via email to