On Tue, 6 Dec 2022 13:02:40 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Doug Simon has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - incorporate review feedback [skip ci]
>> - removed hard-coded module name [skip ci]
>
> src/java.base/share/classes/jdk/internal/vm/TranslatedException.java line 44:
>
>> 42: */
>> 43: @SuppressWarnings("serial")
>> 44: final class TranslatedException extends Exception {
>
> Would it be possible to re-format this to avoid the wildly long (150+) lines?
> This seems to be moving jdk.vm.ci.hotspot.TranslatedException and hard to see
> what is going on.
Is there some tool support for formatting Java source code to meet OpenJDK
coding guidelines? Rather unfortunate that one has to do this manually and
reviewers have to spend time manually checking it.
> src/java.base/share/classes/jdk/internal/vm/VMSupport.java line 66:
>
>> 64: * only contains String keys and values.
>> 65: */
>> 66: private static byte[] serializePropertiesToByteArray(Properties p,
>> boolean filter) throws IOException {
>
> I think we need more context as to why there are non-String system properties
> in use here.
This parameter exists to allow a caller to pass in a `Properties` object that
is guaranteed to have only String keys and so does not need the extra filtering
done in this method. I'll refactor the code to make it clearer.
> src/java.base/share/classes/jdk/internal/vm/VMSupport.java line 106:
>
>> 104: }
>> 105: if (props.get("oome") != null) {
>> 106: throw new OutOfMemoryError("forced OOME");
>
> I don't think code in java.base should be checking for a property named
> "oome". Is this for a test that sets this system property on the command
> line?
Sorry, that's debug code that I will remove.
-------------
PR: https://git.openjdk.org/jdk/pull/11513
