Re: RFR: 8274811: Remove superfluous use of boxing in java.base
On Sat, 11 Sep 2021 12:11:50 GMT, Andrey Turbanov
wrote:
> Usages of primitive types should be preferred and makes code easier to read.
> Similar cleanups:
> 1. [JDK-8273168](https://bugs.openjdk.java.net/browse/JDK-8273168)
> java.desktop
> 2. [JDK-8274234](https://bugs.openjdk.java.net/browse/JDK-8274234)
> java.sql.rowset
src/java.base/share/classes/sun/security/tools/keytool/Main.java line 4135:
> 4133: if (date != null) {
> 4134: if (date.matches("\\d\\d\\d\\d\\/\\d\\d\\/\\d\\d"))
> {
> 4135: c.set(Integer.parseInt(date.substring(0, 4)),
Could this, and several of the other cases identified in this PR, be replaced
with a call to the `Integer.parseInt(CharSequence, int, int, int)` method to
also avoid the String allocation?
Though there are enough other places in the JDK where this opportunity seems
valuable that maybe it warrants a tracking issue of its own.
-
PR: https://git.openjdk.java.net/jdk/pull/5481
Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries
On Fri, 8 Oct 2021 21:29:19 GMT, Maurizio Cimadamore
wrote:
>> Hi @mcimadamore,
>>
>> As you mentioned at
>> https://github.com/openjdk/jdk/pull/4316#issuecomment-853238872, the system
>> lookup is supported by the underlying `NativeLibraries` which also works on
>> OpenJDK16 to support `LibraryLookup::ofDefault`.
>>
>> But my finding is that it `LibraryLookup::ofDefault` works good on AIX (with
>> `libc.a`) in OpenJDK16 but `CLinker.systemLookup()` ended up with
>> `NoSuchElementException` after changing the code in `SystemLookup.java` and
>> `CABI.java` as follows:
>>
>> private static final SymbolLookup syslookup = switch (CABI.current()) {
>> case SysV, LinuxAArch64, MacOsAArch64, AIX -> libLookup(libs ->
>> libs.loadLibrary("syslookup"));
>> case Win64 -> makeWindowsLookup(); // out of line to workaround
>> javac crash
>> };
>>
>> with a simple test & output:
>>
>> import jdk.incubator.foreign.CLinker;
>> import static jdk.incubator.foreign.CLinker.*;
>> import jdk.incubator.foreign.SymbolLookup;
>> import jdk.incubator.foreign.Addressable;
>>
>> public class Test {
>> private static CLinker clinker = CLinker.getInstance();
>> private static final SymbolLookup defaultLibLookup =
>> CLinker.systemLookup();
>>
>> public static void main(String args[]) throws Throwable {
>> Addressable strlenSymbol =
>> defaultLibLookup.lookup("strlen").get();
>> }
>> }
>>
>> bin/java --enable-native-access=ALL-UNNAMED --add-modules
>> jdk.incubator.foreign -Dforeign.restricted=permit Test
>> WARNING: Using incubator modules: jdk.incubator.foreign
>> Exception in thread "main" java.util.NoSuchElementException: No value
>> present <-
>> at java.base/java.util.Optional.get(Optional.java:143)
>> at Test.main(Test.java:11)
>>
>>
>> So I am wondering what happened to the system lookup in such case given
>> there should be no fundamental difference in leveraging `NativeLibraries` (I
>> assume the library loading in OpenJDK16 & 17 is the same at this point)
>> unless there is something else new in OpenJDK17 I am unaware of (e.g. the
>> changes in `Lib.gmk` or `lib-std.m4`, or a custom system lookup is required
>> on AIX, etc). Thanks.
>
>> So I am wondering what happened to the system lookup in such case given
>> there should be no fundamental difference in leveraging `NativeLibraries` (I
>> assume the library loading in OpenJDK16 & 17 is the same at this point)
>> unless there is something else new in OpenJDK17 I am unaware of (e.g. the
>> changes in `Lib.gmk` or `lib-std.m4`, or a custom system lookup is required
>> on AIX, etc). Thanks.
>
> In 17, SystemLookup loads a specific library that is generated at build time
> - which contains all the c stdlib symbols. That's what the Lib.gmk changes
> are for. What I suspect is that the library is not generated at all, or not
> correctly on AIX, which then causes the system lookup to misbehave.
>
> I would start by double checking that you have a file like this:
>
> /lib/libsyslookup.so
>
> And then, if the library exists, check that it has the right dependency; on
> my linux it's an empty library, but which depends on libc, libm and libdl:
>
>
> ldd lib/libsyslookup.so
> linux-vdso.so.1 (0x7fff2bdf7000)
> libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7f35f1def000)
> libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x7f35f1ca)
> libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x7f35f1c9a000)
Hi @mcimadamore,
Here's output of `libsyslookup.so` on AIX:
$ ldd ./lib/libsyslookup.so
./lib/libsyslookup.so needs: <--- nothing here
$ whereis libc
libc: /usr/lib/libc.a /usr/lib/libc128.a /usr/ccs/lib/libc.a
So it is high likely that there were no dependencies in this generated library.
> perhaps on AIX something similar to what we did for Windows [1] would be
> better.
That's what I thought to be the only way around but might need to figure out
the specifics on AIX.
-
PR: https://git.openjdk.java.net/jdk/pull/4316
Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries
On Fri, 8 Oct 2021 04:43:08 GMT, Cheng Jin wrote: > So I am wondering what happened to the system lookup in such case given there > should be no fundamental difference in leveraging `NativeLibraries` (I assume > the library loading in OpenJDK16 & 17 is the same at this point) unless there > is something else new in OpenJDK17 I am unaware of (e.g. the changes in > `Lib.gmk` or `lib-std.m4`, or a custom system lookup is required on AIX, > etc). Thanks. In 17, SystemLookup loads a specific library that is generated at build time - which contains all the c stdlib symbols. That's what the Lib.gmk changes are for. What I suspect is that the library is not generated at all, or not correctly on AIX, which then causes the system lookup to misbehave. I would start by double checking that you have a file like this: /lib/libsyslookup.so And then, if the library exists, check that it has the right dependency; on my linux it's an empty library, but which depends on libc, libm and libdl: ldd lib/libsyslookup.so linux-vdso.so.1 (0x7fff2bdf7000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7f35f1def000) libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x7f35f1ca) libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x7f35f1c9a000) - PR: https://git.openjdk.java.net/jdk/pull/4316
Re: RFR: 8274949: Use String.contains() instead of String.indexOf() in java.base
On Fri, 8 Oct 2021 09:35:25 GMT, Daniel Fuchs wrote: >Did you run tier1, tier2? I did run tier2. (tier1 is automatically checked by GithubActions). No new tests failed. Only _usual_ tests, which always fail on my machine, were failed. - PR: https://git.openjdk.java.net/jdk/pull/5559
Re: RFR: 8274949: Use String.contains() instead of String.indexOf() in java.base
On Fri, 17 Sep 2021 08:56:47 GMT, Andrey Turbanov wrote: > String.contains was introduced in Java 5. > Some code in java.base still uses old approach with `String.indexOf` to check > if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5559
Re: RFR: 8274949: Use String.contains() instead of String.indexOf() in java.base
On Fri, 17 Sep 2021 08:56:47 GMT, Andrey Turbanov wrote: > String.contains was introduced in Java 5. > Some code in java.base still uses old approach with `String.indexOf` to check > if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. java.net changes look OK to me. - Marked as reviewed by vtewari (Committer). PR: https://git.openjdk.java.net/jdk/pull/5559
Re: RFR: 8274949: Use String.contains() instead of String.indexOf() in java.base
On Fri, 17 Sep 2021 08:56:47 GMT, Andrey Turbanov wrote: > String.contains was introduced in Java 5. > Some code in java.base still uses old approach with `String.indexOf` to check > if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. Changes to java.net look OK. Did you run tier1, tier2? - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5559
RFR: 8274949: Use String.contains() instead of String.indexOf() in java.base
String.contains was introduced in Java 5. Some code in java.base still uses old approach with `String.indexOf` to check if String contains specified substring. I propose to migrate such usages. Makes code shorter and easier to read. - Commit messages: - [PATCH] Use String.contains() instead of String.indexOf() in java.base - [PATCH] Use String.contains() instead of String.indexOf() in java.base Changes: https://git.openjdk.java.net/jdk/pull/5559/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5559=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274949 Stats: 40 lines in 17 files changed: 0 ins; 3 del; 37 mod Patch: https://git.openjdk.java.net/jdk/pull/5559.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5559/head:pull/5559 PR: https://git.openjdk.java.net/jdk/pull/5559
Re: RFR: 8274949: Use String.contains() instead of String.indexOf() in java.base
On Fri, 17 Sep 2021 08:56:47 GMT, Andrey Turbanov wrote: > String.contains was introduced in Java 5. > Some code in java.base still uses old approach with `String.indexOf` to check > if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. security-related change looks fine. - Marked as reviewed by weijun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5559
