On Tue, 22 Nov 2022 19:54:51 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
> Now that the javacserver no longer has any ambitions outside being a
> buildtool customized for the JDK build process, a lot of abstractions and
> generalizations can be removed.
>
> This will allow the actual behavior to be more clearly visible, and will help
> debugging the issues we are still seeing (most likely race conditions), and
> to convert the tool to use the ToolProvider API in the future.
Only some minor comments. Looks good to me overall. The proof is in the
building.
Have you tried more than just full builds on all platforms? Things like
incremental builds, some forced failure conditions etc?
make/langtools/tools/javacserver/client/Client.java line 140:
> 138:
> 139: /*
> 140: * Fork a server process process and wait for server to come around
Suggestion:
* Fork a server process and wait for server to come around
make/langtools/tools/javacserver/client/Client.java line 148:
> 146: cmd.addAll(Arrays.asList(conf.javaCommand().split(" ")));
> 147: // javacserver.server.Server is the server main class
> 148: cmd.add("javacserver.server.Server");
Did you consider referencing the class name using `Server.class.getName()`?
make/langtools/tools/javacserver/server/Server.java line 200:
> 198:
> 199: // Set up logging for this thread. Stream back logging
> messages to
> 200: // client on the format format "level:msg".
Suggestion:
// client on the format "level:msg".
-------------
Marked as reviewed by erikj (Reviewer).
PR: https://git.openjdk.org/jdk/pull/11298
