Re: RFR: 8273801: Handle VMTYPE for "core" VM variant

2021-09-15 Thread David Holmes

On 15/09/2021 11:34 pm, Aleksey Shipilev wrote:

On Wed, 15 Sep 2021 12:45:51 GMT, David Holmes  wrote:


But perhaps we should be looking to remove "core" going forward?


Yes, we should consider it. @magicus [told 
me](https://github.com/openjdk/jdk/pull/5440#discussion_r705464312) that somebody wanted 
for "core" to remain, the last time the removal was suggested. I guess is it 
somewhat useful in porting work: producing the builds where only template interpreter is 
implemented.


Good point - I hadn't considered that. But I wonder whether it is 
actually still functional? If so and we want to keep it we should 
probably build it regularly.


David


-

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



Re: RFR: 8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux [v3]

2021-09-15 Thread Philip Race

You are right there's no check. One could be added by a motivated party ..
The minimum for Linux may be as old as 1.2.3  but safer is 2.3.1 since 
we rely on that for AAT font support.
I can't (quickly) speak to any important bug fixes in later releases we 
may need, just API / functionality.


-phi.

On 9/15/21 12:47 PM, Andrew John Hughes wrote:

On Tue, 16 Mar 2021 16:56:22 GMT, Phil Race  wrote:


 From a build perspective this partially reverts 
https://bugs.openjdk.java.net/browse/JDK-8249821 except that it keeps
the harfbuzz sources separate and still supports building and running against a 
system harfbuzz which is only of interest or relevance on Linux.

I ended up having to go this way because its is the least unsatisfactory 
solution.
I did not want us to build a devkit to link against a system linux only to find 
we couldn't use it at runtime
because too many systems have to old a version of harfbuzz.

This solves the Manjaro Linux problem and I've manually verified building 
against a system hardbuxz on Ubuntu 20.10

There are couple of incidental fixes in here too
- "libharfbuzz" should not have been in the EXTRA_HEADERS var when building 
against a system version
- harfbuzz/hb-ucdn is gone and should not be listed as a header directory 
needed to build the bundled copy
- I expect it also resolves https://bugs.openjdk.java.net/browse/JDK-8262502

Phil Race has updated the pull request incrementally with one additional commit 
since the last revision:

   8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux

You mention that "too many systems have to old a version of harfbuzz".
Is the required version defined somewhere? There's no check in configure for 
either a version or a required symbol:
https://github.com/openjdk/jdk/blob/cbffecc61e4a9ac1172926ef4f20d918d73adde9/make/autoconf/lib-bundled.m4#L291

With undefined symbols also being left to runtime (hence why JDK-8272332 
doesn't show up during build), it seems this could lead to sporadic runtime 
failures if OpenJDK is unknowingly built against a harfbuzz that is too old.

-

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




Re: RFR: 8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux [v3]

2021-09-15 Thread Andrew John Hughes
On Tue, 16 Mar 2021 16:56:22 GMT, Phil Race  wrote:

>> From a build perspective this partially reverts 
>> https://bugs.openjdk.java.net/browse/JDK-8249821 except that it keeps 
>> the harfbuzz sources separate and still supports building and running 
>> against a system harfbuzz which is only of interest or relevance on Linux.
>> 
>> I ended up having to go this way because its is the least unsatisfactory 
>> solution.
>> I did not want us to build a devkit to link against a system linux only to 
>> find we couldn't use it at runtime
>> because too many systems have to old a version of harfbuzz.
>> 
>> This solves the Manjaro Linux problem and I've manually verified building 
>> against a system hardbuxz on Ubuntu 20.10
>> 
>> There are couple of incidental fixes in here too
>> - "libharfbuzz" should not have been in the EXTRA_HEADERS var when building 
>> against a system version
>> - harfbuzz/hb-ucdn is gone and should not be listed as a header directory 
>> needed to build the bundled copy
>> - I expect it also resolves https://bugs.openjdk.java.net/browse/JDK-8262502
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8255790: GTKL&F: Java 16 crashes on initialising GTKL&F on Manjaro Linux

You mention that "too many systems have to old a version of harfbuzz".
Is the required version defined somewhere? There's no check in configure for 
either a version or a required symbol:
https://github.com/openjdk/jdk/blob/cbffecc61e4a9ac1172926ef4f20d918d73adde9/make/autoconf/lib-bundled.m4#L291

With undefined symbols also being left to runtime (hence why JDK-8272332 
doesn't show up during build), it seems this could lead to sporadic runtime 
failures if OpenJDK is unknowingly built against a harfbuzz that is too old.

-

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


Re: RFR: 8273797: Stop impersonating "server" VM in all VM variants

2021-09-15 Thread Erik Joelsson
On Wed, 15 Sep 2021 10:02:19 GMT, Aleksey Shipilev  wrote:

> As the follow-up for Zero-specific JDK-8273494, we might want to clean up 
> build system logic for all VM variants: stop impersonating "server" VMs for 
> all of them. This basically leaves "core" and "custom" variants to be handled 
> with this patch.
> 
> Additional testing:
>  - [x] Linux x86_64 `core` build passes, `libjvm.so` now in `core`, `jvm.cfg` 
> mentions `-core KNOWN`
>  - [x] Linux x86_64 `custom` (+serialgc,+compiler2) build passes, `libjvm.so` 
> now in `custom`, `jvm.cfg` mentions `-custom KNOWN`

Marked as reviewed by erikj (Reviewer).

-

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


Re: RFR: 8273801: Handle VMTYPE for "core" VM variant

2021-09-15 Thread Erik Joelsson
On Wed, 15 Sep 2021 09:55:22 GMT, Aleksey Shipilev  wrote:

> Building with --with-jvm-variants=core currently produces a binary that 
> replies an odd version:
> 
> 
> $ build/linux-x86_64-core-fastdebug/images/jdk/bin/java -version
> openjdk version "18-internal" 2022-03-15
> OpenJDK Runtime Environment (fastdebug build 18-internal+0-adhoc.shade.jdk)
> OpenJDK 64-Bit  VM (fastdebug build 18-internal+0-adhoc.shade.jdk, 
> interpreted mode)
> 
> 
> It should actually say "OpenJDK 64-Bit Core VM", I think. Build just misses a 
> simple definition of `VMTYPE` for core variant. 
> 
> After the patch:
> 
> 
> $ build/linux-x86_64-core-fastdebug/images/jdk/bin/java -client -version
> openjdk version "18-internal" 2022-03-15
> OpenJDK Runtime Environment (fastdebug build 18-internal+0-adhoc.shade.jdk)
> OpenJDK 64-Bit Core VM (fastdebug build 18-internal+0-adhoc.shade.jdk, 
> interpreted mode)

Marked as reviewed by erikj (Reviewer).

-

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


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v2]

2021-09-15 Thread Michael McMahon
On Wed, 15 Sep 2021 08:42:40 GMT, Julia Boes  wrote:

>> src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpsServer.java 
>> line 152:
>> 
>>> 150: return server;
>>> 151: }
>>> 152: 
>> 
>> Too bad we couldn't simplify the setting up a basic certificate for https.
>
> That would be nice indeed, but the goal of this JEP is a minimal HTTP-only 
> server, intentionally leaving anything HTTPS aside. `HttpsServer::create` 
> being the exception, added to provide the same convenience as for 
> `HttpServer`. Any HTTPS configuration can be done using the existing API.

I agree the JEP should focus on a minimal HTTP server and the new API does 
allow an HTTPS based file server to be setup in one or two lines of code. I 
don't think there would be much value in providing something like self signed 
certificates, but it has become a lot easier to obtain real https certificates 
through services like LetsEncrypt so it might be interesting to write an 
article for inside.java showing how to set up a HTTPS server from start to 
finish.

-

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


Re: RFR: 8273801: Handle VMTYPE for "core" VM variant

2021-09-15 Thread Aleksey Shipilev
On Wed, 15 Sep 2021 12:45:51 GMT, David Holmes  wrote:

> But perhaps we should be looking to remove "core" going forward?

Yes, we should consider it. @magicus [told 
me](https://github.com/openjdk/jdk/pull/5440#discussion_r705464312) that 
somebody wanted for "core" to remain, the last time the removal was suggested. 
I guess is it somewhat useful in porting work: producing the builds where only 
template interpreter is implemented.

-

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


Re: RFR: 8273797: Stop impersonating "server" VM in all VM variants

2021-09-15 Thread Aleksey Shipilev
On Wed, 15 Sep 2021 12:56:34 GMT, David Holmes  wrote:

> Did you test building all variants into one image?

Yes, I did, but I think the build system is confused about the VM feature sets. 
I have a vague recollection that ether @magicus or someone else at one point 
suggested eliminating multi-variant builds, maybe that should be the next step 
in build simplification.

-

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


Re: RFR: 8273797: Stop impersonating "server" VM in all VM variants

2021-09-15 Thread David Holmes
On Wed, 15 Sep 2021 10:02:19 GMT, Aleksey Shipilev  wrote:

> As the follow-up for Zero-specific JDK-8273494, we might want to clean up 
> build system logic for all VM variants: stop impersonating "server" VMs for 
> all of them. This basically leaves "core" and "custom" variants to be handled 
> with this patch.
> 
> Additional testing:
>  - [x] Linux x86_64 `core` build passes, `libjvm.so` now in `core`, `jvm.cfg` 
> mentions `-core KNOWN`
>  - [x] Linux x86_64 `custom` (+serialgc,+compiler2) build passes, `libjvm.so` 
> now in `custom`, `jvm.cfg` mentions `-custom KNOWN`

>From a purely "mechanical" perspective this is a good simplification and 
>cleanup.

Did you test building all variants into one image?

Cheers,
David

-

Marked as reviewed by dholmes (Reviewer).

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


Re: RFR: 8273801: Handle VMTYPE for "core" VM variant

2021-09-15 Thread David Holmes
On Wed, 15 Sep 2021 09:55:22 GMT, Aleksey Shipilev  wrote:

> Building with --with-jvm-variants=core currently produces a binary that 
> replies an odd version:
> 
> 
> $ build/linux-x86_64-core-fastdebug/images/jdk/bin/java -version
> openjdk version "18-internal" 2022-03-15
> OpenJDK Runtime Environment (fastdebug build 18-internal+0-adhoc.shade.jdk)
> OpenJDK 64-Bit  VM (fastdebug build 18-internal+0-adhoc.shade.jdk, 
> interpreted mode)
> 
> 
> It should actually say "OpenJDK 64-Bit Core VM", I think. Build just misses a 
> simple definition of `VMTYPE` for core variant. 
> 
> After the patch:
> 
> 
> $ build/linux-x86_64-core-fastdebug/images/jdk/bin/java -client -version
> openjdk version "18-internal" 2022-03-15
> OpenJDK Runtime Environment (fastdebug build 18-internal+0-adhoc.shade.jdk)
> OpenJDK 64-Bit Core VM (fastdebug build 18-internal+0-adhoc.shade.jdk, 
> interpreted mode)

Trivially fine.

But perhaps we should be looking to remove "core" going forward?

Thanks.
David

-

Marked as reviewed by dholmes (Reviewer).

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


RFR: 8273797: Stop impersonating "server" VM in all VM variants

2021-09-15 Thread Aleksey Shipilev
As the follow-up for Zero-specific JDK-8273494, we might want to clean up build 
system logic for all VM variants: stop impersonating "server" VMs for all of 
them. This basically leaves "core" and "custom" variants to be handled with 
this patch.

Additional testing:
 - [x] Linux x86_64 `core` build passes, `libjvm.so` now in `core`, `jvm.cfg` 
mentions `-core KNOWN`
 - [x] Linux x86_64 `custom` (+serialgc,+compiler2) build passes, `libjvm.so` 
now in `custom`, `jvm.cfg` mentions `-custom KNOWN`

-

Commit messages:
 - Fix

Changes: https://git.openjdk.java.net/jdk/pull/5526/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5526&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8273797
  Stats: 41 lines in 5 files changed: 3 ins; 33 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5526.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5526/head:pull/5526

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


RFR: 8273801: Handle VMTYPE for "core" VM variant

2021-09-15 Thread Aleksey Shipilev
Building with --with-jvm-variants=core currently produces a binary that replies 
an odd version:


$ build/linux-x86_64-core-fastdebug/images/jdk/bin/java -version
openjdk version "18-internal" 2022-03-15
OpenJDK Runtime Environment (fastdebug build 18-internal+0-adhoc.shade.jdk)
OpenJDK 64-Bit  VM (fastdebug build 18-internal+0-adhoc.shade.jdk, interpreted 
mode)


It should actually say "OpenJDK 64-Bit Core VM", I think. Build just misses a 
simple definition of `VMTYPE` for core variant. 

After the patch:


$ build/linux-x86_64-core-fastdebug/images/jdk/bin/java -client -version
openjdk version "18-internal" 2022-03-15
OpenJDK Runtime Environment (fastdebug build 18-internal+0-adhoc.shade.jdk)
OpenJDK 64-Bit Core VM (fastdebug build 18-internal+0-adhoc.shade.jdk, 
interpreted mode)

-

Commit messages:
 - Fix

Changes: https://git.openjdk.java.net/jdk/pull/5525/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5525&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8273801
  Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5525.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5525/head:pull/5525

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


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v2]

2021-09-15 Thread Julia Boes
On Wed, 15 Sep 2021 03:14:04 GMT, Jaikiran Pai  wrote:

>> FWIW `.z` is the extension of the old Unix `compress` program.
>
>> FWIW `.z` is the extension of the old Unix `compress` program.
> 
> Thank you Florent, I wasn't aware of that.

related PR for reference: https://github.com/openjdk/jdk/pull/5506

-

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


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v2]

2021-09-15 Thread Julia Boes
> This change implements a simple web server that can be run on the 
> command-line with `java -m jdk.httpserver`.
> 
> This is facilitated by adding an entry point for the `jdk.httpserver` module, 
> an implementation class whose main method is run when the above command is 
> executed. This is the first such module entry point in the JDK.
> 
> The server is a minimal HTTP server that serves the static files of a given 
> directory, similar to existing alternatives on other platforms and convenient 
> for testing, development, and debugging.
> 
> Additionally, a small API is introduced for programmatic creation and 
> customization.
> 
> Testing: tier1-3.

Julia Boes has updated the pull request incrementally with three additional 
commits since the last revision:

 - small spec rewording
 - add module main class to symbolgenerator
 - remove UnmodifiableHeaders constant

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5505/files
  - new: https://git.openjdk.java.net/jdk/pull/5505/files/093263d0..f53c59d9

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5505&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5505&range=00-01

  Stats: 32 lines in 3 files changed: 25 ins; 2 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5505.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5505/head:pull/5505

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


Re: RFR: 8273494: Zero: Put libjvm.so into "zero" folder, not "server"

2021-09-15 Thread Aleksey Shipilev
On Thu, 9 Sep 2021 10:17:02 GMT, Aleksey Shipilev  wrote:

> Currently, the build system defaults the libjvm.so location to "server".  
> This makes looking for `libjvm.so` awkward, see JDK-8273487 for example. We 
> need to see if moving the libjvm.so to a proper location breaks anything. 
> 
> Additional testing:
>  - [x] Linux x86_64 Zero build
>  - [x] Linux x86_64 Zero bootcycle-images
>  - [x] Linux x86_64 Zero `tier1`

Thank you all!

-

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


Integrated: 8273494: Zero: Put libjvm.so into "zero" folder, not "server"

2021-09-15 Thread Aleksey Shipilev
On Thu, 9 Sep 2021 10:17:02 GMT, Aleksey Shipilev  wrote:

> Currently, the build system defaults the libjvm.so location to "server".  
> This makes looking for `libjvm.so` awkward, see JDK-8273487 for example. We 
> need to see if moving the libjvm.so to a proper location breaks anything. 
> 
> Additional testing:
>  - [x] Linux x86_64 Zero build
>  - [x] Linux x86_64 Zero bootcycle-images
>  - [x] Linux x86_64 Zero `tier1`

This pull request has now been integrated.

Changeset: 8fbcc823
Author:Aleksey Shipilev 
URL:   
https://git.openjdk.java.net/jdk/commit/8fbcc8239a3fc04e56ebbd287c7bb5db731977b7
Stats: 14 lines in 6 files changed: 0 ins; 3 del; 11 mod

8273494: Zero: Put libjvm.so into "zero" folder, not "server"

Reviewed-by: ihse, sgehwolf

-

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


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

2021-09-15 Thread Julia Boes
On Tue, 14 Sep 2021 15:56:28 GMT, Jim Laskey  wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpsServer.java line 
> 152:
> 
>> 150: return server;
>> 151: }
>> 152: 
> 
> Too bad we couldn't simplify the setting up a basic certificate for https.

That would be nice indeed, but the goal of this JEP is a minimal HTTP-only 
server, intentionally leaving anything HTTPS aside. `HttpsServer::create` being 
the exception, added to provide the same convenience as for `HttpServer`. Any 
HTTPS configuration can be done using the existing API.

-

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


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

2021-09-15 Thread Julia Boes
On Wed, 15 Sep 2021 07:49:38 GMT, Michael McMahon  wrote:

>> Or maybe - which would be more accurate:
>> 
>> 
>> with the given {@code statusCode} and the body bytes' length (or {@code -1} 
>> if the body is empty).
>
> I agree with your second suggestion. It's better not to refer to the 
> 'Content-Length' header at all.

Indeed more accurate, I'll update to the second suggestion.

-

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


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

2021-09-15 Thread Julia Boes
On Tue, 14 Sep 2021 16:45:08 GMT, Daniel Fuchs  wrote:

>> Just a rewording, maybe.
>
> Hmm... Maye that should be "The response headers *are sent*". The non-obvious 
> technical details is that the response headers are sent before the body - as 
> soon as you call  `sendResponseHeaders`. The link is here to provide a better 
> understanding of the workings of the new Handler. The headers are sent by 
> calling `sendResponseHeaders` on the HttpExchange.

Good point on rephrasing to "The response headers are sent", I'll change that.

-

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


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

2021-09-15 Thread Julia Boes
On Tue, 14 Sep 2021 13:38:27 GMT, Daniel Fuchs  wrote:

>> src/jdk.httpserver/share/classes/com/sun/net/httpserver/Headers.java line 
>> 106:
>> 
>>> 104: var h = headers.entrySet().stream()
>>> 105: .collect(Collectors.toUnmodifiableMap(
>>> 106: Entry::getKey, e -> new 
>>> LinkedList<>(e.getValue(;
>> 
>> I wonder, what the reason of  `LinkedList` usages here?
>> As I know ArrayList is faster in all real-world scenarios.
>
> Hi Andrey, IIRC from when I reviewed this code the current implementation of 
> `Headers` already uses `LinkedList` in other places - so this preserves the 
> concrete list implementation class that `Headers` uses (see `Headers::add`). 
> Not that it matters much - but if we wanted to replace `LinkedList` with 
> `ArrayList`  I believe we should do it consistently in this class - and 
> probably outside of the realm of this JEP.

That's right, it's due to the pre-existing implementation.

-

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


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

2021-09-15 Thread Michael McMahon
On Tue, 14 Sep 2021 16:51:40 GMT, Daniel Fuchs  wrote:

>> src/jdk.httpserver/share/classes/com/sun/net/httpserver/HttpHandlers.java 
>> line 129:
>> 
>>> 127:  * response body bytes are a {@code UTF-8} encoded byte 
>>> sequence of
>>> 128:  * {@code body}. The response {@linkplain 
>>> HttpExchange#sendResponseHeaders(int, long) is sent}
>>> 129:  * with the given {@code statusCode} and the body bytes' length. 
>>> The body
>> 
>> That might give the impression that chunked encoding will be used if the 
>> body length is 0. I wonder if it should say instead:
>> 
>> 
>> with the given {@code statusCode} and a {@code Content-Length} field set to 
>> the body bytes' length.
>
> Or maybe - which would be more accurate:
> 
> 
> with the given {@code statusCode} and the body bytes' length (or {@code -1} 
> if the body is empty).

I agree with your second suggestion. It's better not to refer to the 
'Content-Length' header at all.

-

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