Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8308753: Class-File API transition to Preview #15706

Closed
wants to merge 372 commits into from

Conversation

asotona
Copy link
Member

@asotona asotona commented Sep 13, 2023

Classfile API is an internal library under package jdk.internal.classfile in JDK 21.
This pull request turns the Classfile API into a preview feature and moves it into java.lang.classfile.
It repackages all uses across JDK and tests and adds lots of missing Javadoc.

This PR goes in sync with JDK-8308754: Class-File API (Preview) (CSR)
and JDK-8280389: Class-File API (Preview) (JEP).

Online javadoc is available at: 
https://cr.openjdk.org/~asotona/JDK-8308753-preview/api/java.base/java/lang/classfile/package-summary.html

In addition to the primary transition to preview, this pull request also includes:

  • All Classfile* classes ranamed to ClassFile* (based on JEP discussion).
  • A new preview feature, CLASSFILE_API, has been added.
  • Buildsystem tool required a little patch to support annotated modules.
  • All JDK modules using the Classfile API are newly participating in the preview.
  • All tests that use the Classfile API now have preview enabled.
  • A few Javac tests not allowing preview have been partially reverted; their conversion can be re-applied when the Classfile API leaves preview mode.

Despite the number of affected files, this pull request is relatively straight-forward. The preview version of the Classfile API is based on the internal version of the library from the JDK master branch, and there are no API features added.

Please review this pull request to help the Classfile API turn into a preview.

Any comments are welcome.

Thanks,
Adam


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change requires CSR request JDK-8308754 to be approved
  • Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issues

  • JDK-8308753: Class-File API transition to Preview (Sub-task - P4)
  • JDK-8308754: Class-File API (Preview) (CSR)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/15706/head:pull/15706
$ git checkout pull/15706

Update a local copy of the PR:
$ git checkout pull/15706
$ git pull https://git.openjdk.org/jdk.git pull/15706/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 15706

View PR using the GUI difftool:
$ git pr show -t 15706

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/15706.diff

Webrev

Link to Webrev Comment

asotona and others added 30 commits February 15, 2023 16:26
…CodeStackTracker.java

Co-authored-by: Maurizio Cimadamore <54672762+mcimadamore@users.noreply.github.com>
…CodeStackTracker.java

Co-authored-by: Maurizio Cimadamore <54672762+mcimadamore@users.noreply.github.com>
…CodeRelabeler.java

Co-authored-by: Maurizio Cimadamore <54672762+mcimadamore@users.noreply.github.com>
…d to AbstractRefEntry, AbstractRefsEntry and AbstractNamedEntry
…ons to ConstantDynamicEntry, MethodHandleEntry and MethodTypeEntry
@kwin
Copy link

kwin commented Nov 8, 2023

You can simply call ClassFile.of().parse(inputStream.readAllBytes()) instead.

I know, but for memory consumption reasons everyone should prefer just passing an InputStream. Is the memory consumption with ClassFile.of().parse(Path) any better (i.e. are you using a java.io.RandomAccessFile or java.nio.channels.SeekableByteChannel under the hood)? I think that deserves a sentence in the javadoc.

@liach
Copy link
Member

liach commented Nov 9, 2023

I know, but for memory consumption reasons everyone should prefer just passing an InputStream. Is the memory consumption with ClassFile.of().parse(Path) any better (i.e. are you using a java.io.RandomAccessFile or java.nio.channels.SeekableByteChannel under the hood)? I think that deserves a sentence in the javadoc.

FYI parse(Path) is just a shortcut to read all bytes from a file, like parse(Files.readAllBytes(path)). Classfile implementation fetches the byte array very often (constant pool reading, lazy expansion) so keeping the bytes in a in-memory array is better.

@openjdk openjdk bot added merge-conflict Pull request has merge conflict with target branch and removed rfr Pull request is ready for review labels Nov 20, 2023
# Conflicts:
#	test/langtools/tools/javac/7199823/InnerClassCannotBeVerified.java
@openjdk openjdk bot added rfr Pull request is ready for review and removed merge-conflict Pull request has merge conflict with target branch labels Nov 20, 2023
int JAVA_21_VERSION = 65;

/** 66 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a more scalable way to express the major class version? The current approach means we have to add two fields per year. Maybe a lookup function would be better?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is explicit list of supported class file versions, so I don't see any other way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it not be possible to expose an immutable Map<Integer, Integer> that maps from Java version to major class version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually out of scope of this PR. Feel free to request a new ClassFile API feature.

# Conflicts:
#	src/java.base/share/classes/jdk/internal/javac/PreviewFeature.java
@openjdk
Copy link

openjdk bot commented Dec 4, 2023

@asotona This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8308753: Class-File API transition to Preview

Reviewed-by: ihse, mchung, vromero

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added ready Pull request is ready to be integrated and removed csr Pull request needs approved CSR before integration labels Dec 4, 2023
@asotona
Copy link
Member Author

asotona commented Dec 4, 2023

/integrate

@openjdk
Copy link

openjdk bot commented Dec 4, 2023

Going to push as commit 2b00ac0.

@openjdk openjdk bot added the integrated Pull request has been integrated label Dec 4, 2023
@openjdk openjdk bot closed this Dec 4, 2023
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Dec 4, 2023
@openjdk
Copy link

openjdk bot commented Dec 4, 2023

@asotona Pushed as commit 2b00ac0.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build build-dev@openjdk.org compiler compiler-dev@openjdk.org core-libs core-libs-dev@openjdk.org integrated Pull request has been integrated javadoc javadoc-dev@openjdk.org kulla kulla-dev@openjdk.org
Development

Successfully merging this pull request may close these issues.

10 participants