• Type: Enhancement
• Component: core-libs
• Sub-Component: java.lang
• Affected Version: 15

• Priority: P3
• Status: Resolved
• Resolution: Fixed

• Submitted: 2020-06-12
• Updated: 2024-10-17
• Resolved: 2020-06-19

JDK 15	JDK 16
15 b29Fixed	16Fixed

Changeset: f2b191a6 Author: Mandy Chung <mchung@openjdk.org> Date: 2020-06-19 08:27:59 +0000 URL: https://git.openjdk.java.net/lanai/commit/f2b191a6 02-07-2020 Changeset: f2b191a6 Author: Mandy Chung <mchung@openjdk.org> Date: 2020-06-19 08:27:59 +0000 URL: https://git.openjdk.java.net/panama-foreign/commit/f2b191a6 02-07-2020 Changeset: f2b191a6 Author: Mandy Chung <mchung@openjdk.org> Date: 2020-06-19 08:27:59 +0000 URL: https://git.openjdk.java.net/amber/commit/f2b191a6 02-07-2020 Changeset: f2b191a6 Author: Mandy Chung <mchung@openjdk.org> Date: 2020-06-19 08:27:59 +0000 URL: https://git.openjdk.java.net/mobile/commit/f2b191a6 02-07-2020 URL: https://hg.openjdk.java.net/jdk/jdk15/rev/0385994efa48 User: mchung Date: 2020-06-19 15:28:21 +0000 19-06-2020 [~mr] sure that will be a nice addition 18-06-2020 Late enhancement approved by Project Lead. [~vromero] Please consider updating JEP 384 to mention this change. 18-06-2020 Review thread: https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-June/067249.html Reviewed-by: jrose, dholmes, forax, coleenp, vlivanov CSR has been approved. It is ready to be integrated any time on 6/17/2020 and after. 17-06-2020 Late Enhancement Request: Webrev: http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8247444/webrev.00/ CSR: https://bugs.openjdk.java.net/browse/JDK-8247517 This proposes to make final fields in records not modifiable via reflection. `Field::set` throws IAE if a Field is not modifiable. No change in Field::setAccessible(true), i.e. it will succeed to allow existing frameworks to have read access to final fields in records (no write access). VarHandle does not support write access if it's a static final field or an instance final field. No change in VarHandle. This patch also proposes `sun.misc.Unsafe::objectFieldOffset` and `sun.misc.Unsafe::staticField{Offset/Base}` not to support records. No change is made in JNI. JNI could be considered to disallow modification of final fields in records and hidden classes (static and instance fields). But JNI has superpower and the current spec already allows to modify the value of a final static field even after it's constant folded (via JNI Set<type>Field and SetStatic<type>Field), then all bets are off. This should be re-visited when we consider "final is truly final" for all classes. Make final fields in records not modifiable via reflection enables JIT optimization as these final fields are effectively truly final. This change impacts 3rd-party frameworks including 3rd-party serialization framework that rely on core reflection `setAccessible` or `sun.misc.Unsafe::allocateInstance` and `objectFieldOffset` etc to construct records but not using the canonical constructor. These frameworks would need to be updated to construct records via its canonical constructor as done by the Java serialization. I see this change gives a good opportunity to engage the maintainers of the serialization frameworks and work together to support new features including records, inline classes and the new serialization mechanism and which I think it is worth the investment. This is a low risk enhancement. I'd like to request approval for a late enhancement in JDK 15. It extends the pre-existing code path with refactoring the hidden classes to prepare for new kinds of classes with trusted final fields. The change is straight-forward. Can this wait to integrate in JDK 16? It's important to get this enhancement in when record is a preview feature that we can get feedback and give time to 3rd-party serialization frameworks to look into adding the support for records. If we delayed this change in 16 and records exit preview, it would be bad for frameworks if they verify that they support records in 15 but fail in 16. OTOH the risk of this patch is low and getting this in 15 will get those frameworks which patch the final fields in records notified via exception and follow with actions (either update the framework to support record or send feedback for discussion before records exit preview) Performance Impact I addressed the performance concern I raised earlier myself. For reflection, VM creates the reflective Field objects and fills in MemberName when resolving a member. VM will tag if this Field/MemberName is trusted final field. I think this is a cleaner approach rather than in each place to check for final static and final fields in hidden or record class to determine if it has write access or not. `sun.misc.Unsafe` does not use Field::isTrustedFinalField because (1) Unsafe has been allowing access of static final fields of any classes but isTrustedFinalField is not limited to instance fields (2) Unsafe disallows access to all fields in a hidden class (not limited to trusted final fields). So it follows the precedence and simply checks if the declaring class is a record. `Class::isRecord` calls `Class::getSuperclass` to check if it's a subtype of `Record`. As `Class::getSuperclass` is intrinsified, the call on isRecord on a normal class is fast. Christoph has contributed the microbenchmarks that confirm that no performance regression. 15-06-2020 Suggested patch: http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8247444/webrev.00/ 13-06-2020