Re: Hermetic Java (static image packaging/formatting) investigation and proposal
Hi Magnus, Follow up on your suggestion regarding contributing the minor JDK static build fixes/enhancements (branched from the hermetic Java discussion [1]). Since JEP 178 [2] has covered specification related issues already, we can create individual bugs/enhancements and PRs for the related changes on an as-needed basis. I've created JDK-8303796 [3] as a starting one. Will work with you and others on contributing our changes. Thank you! [1] - https://mail.openjdk.org/pipermail/leyden-dev/2023-February/000119.html [2] - https://openjdk.org/jeps/178 [3] - https://bugs.openjdk.org/browse/JDK-8303796 Best, Jiangli On Tue, Feb 14, 2023 at 12:55 AM Magnus Ihse Bursie < magnus.ihse.bur...@oracle.com> wrote: > On 2023-02-14 00:52, Jiangli Zhou wrote: > > Hi Magnus, > > Thanks for the thoughts! Please see comments inlined below. > > On Mon, Feb 13, 2023 at 5:43 AM Magnus Ihse Bursie < > magnus.ihse.bur...@oracle.com> wrote: > >> Hi Jiangli, >> >> On 2023-02-08 03:08, Jiangli Zhou wrote: >> > Hi Brian, >> > >> > Here are the main buckets of the changes discovered in JDK/VM to >> > support the proposed hermetic image: >> > >> > 1) Resolve symbol conflicts to fully support JDK static builds. Those >> > are mainly caused by duplicated symbols defined in different native >> > libraries or VM code. >> > >> > 2) Complete the built-in native library support in JDK. For easier and >> > more reliable testing/release/deployment, we wanted to support JDK >> > dynamic and static builds with the same set of object files (.o). >> > We've changed to use unique names for >> > JNI_OnLoad|JNI_OnUnload|Agent_OnLoad|Agent_OnUnload|Agent_OnAttach in >> > different JDK JNI libraries by default. For both dynamic linked and >> > static linked JDK builds, we use unique symbols for JNI_OnLoad >> > function and friends. However, non-builtin application JNI libraries >> > can still have the default JNI_OnLoad|... naming. We still properly >> > support application JNI libraries using the default JNI_OnLoad (and >> > friends) naming. >> > >> > As we wanted to produce dynamic and static builds from the same set of >> > object files, we've moved away from using the STATIC_BUILD macro. >> > >> > We've also done some makefile work to build both dynamic shared >> > libraries (DSOs) and static libraries, within one JDK build. >> >> This sounds like interesting work indeed. However, I am inclined to >> agree with Andrew and wonder how much it relates to Project Leyden. It >> might be that Leyden will need some kind of packaging story, and that >> this can have a role to play in that. But it is not immediately clear >> that it does fit in, and indeed, I think this is not one of Leyden main >> problem areas at the time. >> >> But your code sounds very much interesting from a pure build >> perspective! For at least this part of the code, I think you should >> ignore Leyden for now, and just see if the static build changes you have >> made could be fit for inclusion in OpenJDK. >> >> The static build part of the build system has been sadly neglected due >> to resource limitations, for a long time. :( The rudimentary system >> (actually, more like two separate systems) we have was put in place >> mostly due to external requirements from Project Mobile and the Graal >> integration, and was tacked on mostly as an after-thought. It is not >> regularly tested, and I'd frankly be surprised if it actually works >> right now. So I fully understand if you have been staying away from >> STATIC_BUILD. :) >> >> It sounds like you have created a more dynamic system to be able to >> select per library, if it should be compiled statically or dynamically. >> Do I understand you correctly? If done correctly, it can probably help >> bring a better abstraction to the build process. >> > > For JDK/hotspot natives, our experiment/prototype builds all the JDK > regular artifacts (e.g. lib/.../*.so) that the existing JDK build produces. > Additionally, it also creates the JDK static libraries (e.g. > lib_static/*.a) and a bin/javastatic (with most of the needed JDK static > libraries statically linked into the launcher, for testing purposes only, > such as running jtreg tests) in the binary, as part of the single JDK > build. The hermetic image creation is done as a post process, which takes > the needed pre-built JDK static libraries for linking into the final > executable. The post process is done outside the JDK build. The JDK runtime > support is enhanced to be able to support both built-in libraries and > dynamically loaded shared libraries. > > It doesn't seem to be problematic to get the JDK static support into > OpenJDK first. It's especially helpful for you or erikj@ to look at the > makefiles changes and help massage the changes according to the JDK build > standard. > > >> >> If you are willing to contribute your work to OpenJDK, I would >> definitely be interested in studying it in detail. > > > Thanks! > > >> As you might be >> aware, contributions to
Integrated: 8303799: [BACKOUT] JDK-8302801 Remove fdlibm C sources
The message from this sender included one or more files which could not be scanned for virus detection; do not open these files unless you are certain of the sender's intent. -- On Wed, 8 Mar 2023 02:27:21 GMT, David Holmes wrote: > This reverts commit b5b5cba7feb0e7ef957fd6bef1e591fdb6fdaa9f. > > Thanks. This pull request has now been integrated. Changeset: 21a6ab1e Author:David Holmes URL: https://git.openjdk.org/jdk/commit/21a6ab1e3ea5228a31955d58fe75e5ae66d1c6cd Stats: 6643 lines in 65 files changed: 6613 ins; 20 del; 10 mod 8303799: [BACKOUT] JDK-8302801 Remove fdlibm C sources Reviewed-by: darcy, bpb - PR: https://git.openjdk.org/jdk/pull/12916
Integrated: 8302189: Mark assertion failures noreturn
On Fri, 3 Mar 2023 04:04:25 GMT, Kim Barrett wrote: > Also 8302799: Refactor Debugging variable usage for noreturn crash reporting This pull request has now been integrated. Changeset: 5fa9bd45 Author:Kim Barrett URL: https://git.openjdk.org/jdk/commit/5fa9bd458232a0b5f31b1e7e5a4a2b1f4047da35 Stats: 165 lines in 8 files changed: 125 ins; 19 del; 21 mod 8302189: Mark assertion failures noreturn 8302799: Refactor Debugging variable usage for noreturn crash reporting Reviewed-by: dholmes, coleenp - PR: https://git.openjdk.org/jdk/pull/12845
Re: RFR: 8303799: [BACKOUT] JDK-8302801 Remove fdlibm C sources
On Wed, 8 Mar 2023 02:31:09 GMT, Joe Darcy wrote: >> This reverts commit b5b5cba7feb0e7ef957fd6bef1e591fdb6fdaa9f. >> >> Thanks. > > Sorry for the noise (and missing IEEERemainer)! Thanks @dholmes-ora . Thanks for the reviews @jddarcy and @bplb ! - PR: https://git.openjdk.org/jdk/pull/12916
Re: RFR: 8303799: [BACKOUT] JDK-8302801 Remove fdlibm C sources
On Wed, 8 Mar 2023 02:27:21 GMT, David Holmes wrote: > This reverts commit b5b5cba7feb0e7ef957fd6bef1e591fdb6fdaa9f. > > Thanks. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/12916
Re: RFR: 8303799: [BACKOUT] JDK-8302801 Remove fdlibm C sources
On Wed, 8 Mar 2023 02:27:21 GMT, David Holmes wrote: > This reverts commit b5b5cba7feb0e7ef957fd6bef1e591fdb6fdaa9f. > > Thanks. Sorry for the noise (and missing IEEERemainer)! Thanks @dholmes-ora . - Marked as reviewed by darcy (Reviewer). PR: https://git.openjdk.org/jdk/pull/12916
RFR: 8303799: [BACKOUT] JDK-8302801 Remove fdlibm C sources
This reverts commit b5b5cba7feb0e7ef957fd6bef1e591fdb6fdaa9f. Thanks. - Commit messages: - Revert "8302801: Remove fdlibm C sources" Changes: https://git.openjdk.org/jdk/pull/12916/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12916=00 Issue: https://bugs.openjdk.org/browse/JDK-8303799 Stats: 6643 lines in 65 files changed: 6613 ins; 20 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/12916.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12916/head:pull/12916 PR: https://git.openjdk.org/jdk/pull/12916
Re: RFR: 8302189: Mark assertion failures noreturn [v2]
On Tue, 7 Mar 2023 01:36:56 GMT, David Holmes wrote: >> Kim Barrett has updated the pull request incrementally with one additional >> commit since the last revision: >> >> make Debugging::_enabled a nesting counter > > Okay other than the one outstanding minor syntax query, I have nothing > further to add. > > Thanks. Thanks for reviews @dholmes-ora and @coleenp - PR: https://git.openjdk.org/jdk/pull/12845
Re: RFR: 8302189: Mark assertion failures noreturn [v3]
> Also 8302799: Refactor Debugging variable usage for noreturn crash reporting Kim Barrett has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision: - Merge branch 'master' into noreturn2 - make Debugging::_enabled a nesting counter - new implementation of Debugging - noreturn attributes - Changes: - all: https://git.openjdk.org/jdk/pull/12845/files - new: https://git.openjdk.org/jdk/pull/12845/files/f296ab62..59295614 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12845=02 - incr: https://webrevs.openjdk.org/?repo=jdk=12845=01-02 Stats: 18174 lines in 538 files changed: 7268 ins; 8749 del; 2157 mod Patch: https://git.openjdk.org/jdk/pull/12845.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12845/head:pull/12845 PR: https://git.openjdk.org/jdk/pull/12845
Re: RFR: 8302189: Mark assertion failures noreturn [v2]
The message from this sender included one or more files which could not be scanned for virus detection; do not open these files unless you are certain of the sender's intent. -- On Tue, 7 Mar 2023 13:08:22 GMT, Coleen Phillimore wrote: >> Kim Barrett has updated the pull request incrementally with one additional >> commit since the last revision: >> >> make Debugging::_enabled a nesting counter > > src/hotspot/share/utilities/debug.hpp line 108: > >> 106: // because we need a fallback when we don't have any mechanism for >> detecting >> 107: // constant evaluation. >> 108: #if defined(TARGET_COMPILER_gcc) || defined(TARGET_COMPILER_xlc) > > All this seems like it should go in COMPILER_HEADER(globalDefinitions.hpp) > but since globalDefinitions.hpp includes debug.hpp, you can't do this. Can > we file an RFE to clean this up (if possible)? I considered that, and had two reasons for rejecting it. The include ordering problem was one. There are uses of assert and the like in globalDefinitions. Some of those might someday be moved elsewhere, but for now the order is a problem. The other is that this is really not "general purpose", but rather very specifically tailored to the use here. And it can't be general purpose, because there isn't a general fallback for testing for constexpr evaluation. I could have added `HAS_IS_CONSTANT_EVALUATED` and `::is_constant_evaluated()`, with all uses of the latter requiring protection by the former. But I couldn't think of any places where I would want to use that. Using it in, for example, count_trailing_zeros to allow it to be (sometimes) constexpr would make the question of whether it's constexpr be platform-dependent, which means it can't be used in shared required constexpr contexts. And it's even worse since the platform dependency is also currently compiler version dependent. I think that path leads nowhere good. However, I've filed this cleanup RFE: https://bugs.openjdk.org/browse/JDK-8303797 - PR: https://git.openjdk.org/jdk/pull/12845
Re: Integrated: JDK-8302801: Remove fdlibm C sources
Please note this fix has a problem with a missing definition for IEEEremainder that is causing UnsatisfiedLinkError. You can expect some significant noise in testing above tier 1 until this is fixed - which will hopefully be in the next 30 minutes or so. Otherwise a Backout will be performed. David On 8/03/2023 8:31 am, Joe Darcy wrote: On Thu, 2 Mar 2023 05:54:52 GMT, Joe Darcy wrote: While the review of https://github.com/openjdk/jdk/pull/12800 finishes up, I thought I'd get out for the review the next phase of the FDLIBM port: removing the FDLIBM C sources from the repo. A repo with the changes for JDK-8302027 and this PR successful build on the default set of platform and successfully run tier 1 tests, which includes tests of the math library. There are a few remaining references to the case-independent string "fdlibm" in the make directory and HotSpot sources. HotSpot contains a partial fork for FDLIBM (a tine of FDLIBM?) to use for intrinsics. The remaining make machinery contains logic to determine what set of gcc options can be used for the compile. The intention of this change is to remove use of FDLIBM for the core libraries. This pull request has now been integrated. Changeset: b5b5cba7 Author:Joe Darcy URL: https://git.openjdk.org/jdk/commit/b5b5cba7feb0e7ef957fd6bef1e591fdb6fdaa9f Stats: 6643 lines in 65 files changed: 20 ins; 6613 del; 10 mod 8302801: Remove fdlibm C sources Reviewed-by: bpb, dholmes, alanb, kvn - PR: https://git.openjdk.org/jdk/pull/12821
RFR: JDK-8301998: Update HarfBuzz to 7.0.1
HarfBuzz library updated from v4.4.1 to v7.0.1 - harfbuzz.md file updated - Added -DHB_NO_VISIBILITY to HARFBUZZ_CFLAGS in Awt2DLibraries.gmk to avoid build issues on linux and macos targets. - GPOS.hh moved to Layout to Layout/GPOS to match upstream changes. - hb-ot-color-* files moved to OT/Color to match upstream changes. - 41 newly added, 8 deleted and 224 modified files (src + harfbuzz.md). - Commit messages: - updated harfbuzz.md - newly added files - modified files - deleted files Changes: https://git.openjdk.org/jdk/pull/12913/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12913=00 Issue: https://bugs.openjdk.org/browse/JDK-8301998 Stats: 41531 lines in 271 files changed: 24473 ins; 11216 del; 5842 mod Patch: https://git.openjdk.org/jdk/pull/12913.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12913/head:pull/12913 PR: https://git.openjdk.org/jdk/pull/12913
Integrated: 8286781: Replace the deprecated/obsolete gethostbyname and inet_addr calls
On Thu, 2 Mar 2023 22:35:17 GMT, David Holmes wrote: > We can replace `gethostbyname`, which is deprecated on Windows and Linux, > with `getaddrinfo`. This API is available on all supported platforms and so > can be placed in shared code. @djelinski pointed out that `getaddrinfo` can > resolve both IP addresses and host names so the two step approach used in > `networkStream::connect` is not necessary and we can do away with > `os::get_host_by_name()` completely. > > The build is updated to enable winsock deprecation warnings, and now we need > to use `ws2_32.lib` we can drop `wsock32.lib` (as it is basically a subset - > again thanks @djelinski ). > > Testing > - all Oracle builds in tiers 1-5 > - All GHA builds > > The actual code change has to be manually tested because the code is only > used by Ideal Graph Printing to connect to the Ideal Graph Visualizer. I've > manually tested on Windows and Linux and @tobiasholenstein tested macOS. > > Thanks. This pull request has now been integrated. Changeset: d7298245 Author:David Holmes URL: https://git.openjdk.org/jdk/commit/d7298245d6759f62e253b5cf0df975db17fdbf82 Stats: 41 lines in 6 files changed: 13 ins; 17 del; 11 mod 8286781: Replace the deprecated/obsolete gethostbyname and inet_addr calls Co-authored-by: Daniel Jeliński Reviewed-by: kbarrett, djelinski - PR: https://git.openjdk.org/jdk/pull/12842
Withdrawn: JDK-8301998: Update HarfBuzz to 7.0.1
On Fri, 3 Mar 2023 19:36:29 GMT, Harshitha Onkar wrote: > HarfBuzz library updated from v4.4.1 to v7.0.1 > > - harfbuzz.md file updated > - Added -DHB_NO_VISIBILITY to HARFBUZZ_CFLAGS in Awt2DLibraries.gmk to avoid > build issues on linux and macos targets. > - GPOS.hh has been moved from Layout to Layout/GPOS to match upstream changes. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/12864
Re: RFR: JDK-8301998: Update HarfBuzz to 7.0.1 [v3]
On Tue, 7 Mar 2023 00:36:18 GMT, Harshitha Onkar wrote: >> HarfBuzz library updated from v4.4.1 to v7.0.1 >> >> - harfbuzz.md file updated >> - Added -DHB_NO_VISIBILITY to HARFBUZZ_CFLAGS in Awt2DLibraries.gmk to avoid >> build issues on linux and macos targets. >> - GPOS.hh has been moved from Layout to Layout/GPOS to match upstream >> changes. > > Harshitha Onkar has updated the pull request incrementally with one > additional commit since the last revision: > > deleted files Will be creating a new PR for harfbuzz update. - PR: https://git.openjdk.org/jdk/pull/12864
Integrated: JDK-8302801: Remove fdlibm C sources
On Thu, 2 Mar 2023 05:54:52 GMT, Joe Darcy wrote: > While the review of https://github.com/openjdk/jdk/pull/12800 finishes up, I > thought I'd get out for the review the next phase of the FDLIBM port: > removing the FDLIBM C sources from the repo. > > A repo with the changes for JDK-8302027 and this PR successful build on the > default set of platform and successfully run tier 1 tests, which includes > tests of the math library. > > There are a few remaining references to the case-independent string "fdlibm" > in the make directory and HotSpot sources. HotSpot contains a partial fork > for FDLIBM (a tine of FDLIBM?) to use for intrinsics. The remaining make > machinery contains logic to determine what set of gcc options can be used for > the compile. > > The intention of this change is to remove use of FDLIBM for the core > libraries. This pull request has now been integrated. Changeset: b5b5cba7 Author:Joe Darcy URL: https://git.openjdk.org/jdk/commit/b5b5cba7feb0e7ef957fd6bef1e591fdb6fdaa9f Stats: 6643 lines in 65 files changed: 20 ins; 6613 del; 10 mod 8302801: Remove fdlibm C sources Reviewed-by: bpb, dholmes, alanb, kvn - PR: https://git.openjdk.org/jdk/pull/12821
Re: RFR: 8295884: Implement IDE support for Eclipse [v39]
> Eclipse is a popular and very well-known IDE in the world of Java > development, utilized widely in many contexts, by beginners and experienced > teams alike. Although a relatively lightweight IDE, it features surprisingly > powerful indexing and code analysis capabilities, as well as useful tools, > among which are make integration. While the tools it provides are not always > as sophisticated as other IDEs (IntelliJ IDEA will likely come to mind as one > such competitor), the simplicity of using it, as well as the reliability of > this rugged IDE makes up greatly for the slightly less advanced tooling. > Eclipse requires very little starting infrastructure in the workspace for all > these features and indexing support as well, which makes it a good candidate > for developing on the JDK. > > This enhancement adds 4 extra targets to the make system for generating a > basic Eclipse Workspace that provides almost full indexing support for the > JDK, with varying levels as desired, from a minimalistic option only > including the Java Virtual Machine's source code, to generating a workspace > with both Java and C/C++ natures included, which allows for using Eclipse's > unique ability to quickly swap between Java and C/C++ mode to work on both > native and Java sources at the same time. Cross Compiling support is > available, and in its entirety the change touches very little of the existing > make system, barring its own Makefile within the ide subdirectory. > > Indexing capabilities utilizing the enhancement: > src="https://user-images.githubusercontent.com/32636402/197784819-67ec7de4-7e27-4f33-b738-59b75a9e4403.PNG;> > src="https://user-images.githubusercontent.com/32636402/197784843-df8621a8-7b0a-42da-86f4-3afb92de0f71.PNG;> Julian Waters has updated the pull request incrementally with one additional commit since the last revision: Address Review and add extra comments - Changes: - all: https://git.openjdk.org/jdk/pull/10853/files - new: https://git.openjdk.org/jdk/pull/10853/files/13c0b9b9..2ceb46a0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10853=38 - incr: https://webrevs.openjdk.org/?repo=jdk=10853=37-38 Stats: 20 lines in 4 files changed: 13 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/10853.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10853/head:pull/10853 PR: https://git.openjdk.org/jdk/pull/10853
RFR: 8303760: Visual Studio should use the primary variant in the IDE
Currently support for Visual Studio development always assumes server as the variant to use. More accurately this should instead be set to the primary variant instead - Commit messages: - Visual Studio should use the primary variant in the IDE Changes: https://git.openjdk.org/jdk/pull/12906/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12906=00 Issue: https://bugs.openjdk.org/browse/JDK-8303760 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/12906.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12906/head:pull/12906 PR: https://git.openjdk.org/jdk/pull/12906
Re: RFR: 8295884: Implement IDE support for Eclipse [v38]
The message from this sender included one or more files which could not be scanned for virus detection; do not open these files unless you are certain of the sender's intent. -- On Tue, 7 Mar 2023 16:14:57 GMT, Magnus Ihse Bursie wrote: >> Julian Waters has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Wording > > make/ide/visualstudio/hotspot/CreateVSProject.gmk line 38: > >> 36: # able to extract flags, but we do not wish to execute the rules. >> 37: >> 38: # Use primary variant for defines and includes > > This might be a correct fix but it is independent of this PR. Submitted https://bugs.openjdk.org/browse/JDK-8303760 and https://github.com/openjdk/jdk/pull/12906 to fix that issue separately - PR: https://git.openjdk.org/jdk/pull/10853
Re: RFR: 8295884: Implement IDE support for Eclipse [v38]
On Tue, 7 Mar 2023 16:14:32 GMT, Magnus Ihse Bursie wrote: >> Julian Waters has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Wording > > make/ide/eclipse/workspace.template line 4: > >> 2: >> 3: Java >> 4: The official Java HotSpot Virtual Machine, Runtime >> Environment, and Development Kit > > Just call it OpenJDK; there is a legal trademark swamp associated with "Java". Got it, will also rename it to just jdk to match the cloned repository to be extra safe - PR: https://git.openjdk.org/jdk/pull/10853
Re: RFR: 8294982: Implementation of Classfile API [v55]
> This is root pull request with Classfile API implementation, tests and > benchmarks initial drop into JDK. > > Following pull requests consolidating JDK class files parsing, generating, > and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) > will chain to this one. > > Classfile API development is tracked at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch > > Development branch of consolidated JDK class files parsing, generating, and > transforming is at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch > > Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online > API > documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html) > is also available. > > Please take you time to review this non-trivial JDK addition. > > Thank you, > Adam Adam Sotona has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 183 commits: - Merge branch 'master' into JDK-8294982 - fixed new lines at end of file - package jdk.internal.classfile.jdktypes moved to jdk.internal.classfile.java.lang.constant - fixed CodeRelabeler javadoc - Shared `toString` formats for bound and unbound instructions - generic implementation of ResolvedTransform - snippets and tests synced with jdk.jfr class instrumentation source code - simplified initialization of terminal builder in chained builders - simplified CodeImpl.SINGLETON_INSTRUCTIONS initialization - fixed handling of array descriptors by Util::toInternalName - ... and 173 more: https://git.openjdk.org/jdk/compare/45a616a8...4680572a - Changes: https://git.openjdk.org/jdk/pull/10982/files Webrev: https://webrevs.openjdk.org/?repo=jdk=10982=54 Stats: 53052 lines in 322 files changed: 53048 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/10982.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982 PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v54]
> This is root pull request with Classfile API implementation, tests and > benchmarks initial drop into JDK. > > Following pull requests consolidating JDK class files parsing, generating, > and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) > will chain to this one. > > Classfile API development is tracked at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch > > Development branch of consolidated JDK class files parsing, generating, and > transforming is at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch > > Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online > API > documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html) > is also available. > > Please take you time to review this non-trivial JDK addition. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: fixed new lines at end of file - Changes: - all: https://git.openjdk.org/jdk/pull/10982/files - new: https://git.openjdk.org/jdk/pull/10982/files/a994c572..f14287d0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10982=53 - incr: https://webrevs.openjdk.org/?repo=jdk=10982=52-53 Stats: 13 lines in 13 files changed: 0 ins; 0 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/10982.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982 PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8295884: Implement IDE support for Eclipse [v38]
On Mon, 6 Mar 2023 13:08:57 GMT, Julian Waters wrote: >> Eclipse is a popular and very well-known IDE in the world of Java >> development, utilized widely in many contexts, by beginners and experienced >> teams alike. Although a relatively lightweight IDE, it features surprisingly >> powerful indexing and code analysis capabilities, as well as useful tools, >> among which are make integration. While the tools it provides are not always >> as sophisticated as other IDEs (IntelliJ IDEA will likely come to mind as >> one such competitor), the simplicity of using it, as well as the reliability >> of this rugged IDE makes up greatly for the slightly less advanced tooling. >> Eclipse requires very little starting infrastructure in the workspace for >> all these features and indexing support as well, which makes it a good >> candidate for developing on the JDK. >> >> This enhancement adds 4 extra targets to the make system for generating a >> basic Eclipse Workspace that provides almost full indexing support for the >> JDK, with varying levels as desired, from a minimalistic option only >> including the Java Virtual Machine's source code, to generating a workspace >> with both Java and C/C++ natures included, which allows for using Eclipse's >> unique ability to quickly swap between Java and C/C++ mode to work on both >> native and Java sources at the same time. Cross Compiling support is >> available, and in its entirety the change touches very little of the >> existing make system, barring its own Makefile within the ide subdirectory. >> >> Indexing capabilities utilizing the enhancement: >> > src="https://user-images.githubusercontent.com/32636402/197784819-67ec7de4-7e27-4f33-b738-59b75a9e4403.PNG;> >> > src="https://user-images.githubusercontent.com/32636402/197784843-df8621a8-7b0a-42da-86f4-3afb92de0f71.PNG;> > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Wording make/ide/eclipse/workspace.template line 4: > 2: > 3: Java > 4: The official Java HotSpot Virtual Machine, Runtime > Environment, and Development Kit Just call it OpenJDK; there is a legal trademark swamp associated with "Java". make/ide/visualstudio/hotspot/CreateVSProject.gmk line 38: > 36: # able to extract flags, but we do not wish to execute the rules. > 37: > 38: # Use primary variant for defines and includes This might be a correct fix but it is independent of this PR. - PR: https://git.openjdk.org/jdk/pull/10853
Re: RFR: 8294982: Implementation of Classfile API [v49]
On Tue, 7 Mar 2023 15:48:22 GMT, Maurizio Cimadamore wrote: >> Here I'm also not sure I understand, the long line has bee wrapped. > > I mean here (and in the other) there seems to be a missing newline at the end > of the file OK, I'll fix it, thanks. - PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v43]
On Tue, 7 Mar 2023 14:35:22 GMT, Adam Sotona wrote:
>> src/java.base/share/classes/jdk/internal/classfile/impl/EntryMap.java line
>> 176:
>>
>>> 174: }
>>> 175:
>>> 176: public static long nextPowerOfTwo( long x ) {
>>
>> If you like you can change the implementation to be:
>>
>> x = -1 >>> Long.numberOfLeadingZeros(x - 1);
>> return x + 1;
>>
>> See `ConcurrentHashMap`.
>
> `Long:numberOfLeadingZeros` implementation uses more method calls,
> conditional statements and binary operations. I would prefer to stick with
> the current implementation for its speed and simplicity.
`numberOfLeadingZeros` is an intrinsic, so it will compile down to a machine
instruction. Up to you.
-
PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8295884: Implement IDE support for Eclipse [v38]
On Tue, 25 Oct 2022 22:17:25 GMT, Erik Joelsson wrote: >> Julian Waters has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Wording > > This looks like nice work. > > I'm curious how does this eclipse project figures out preprocessor settings > like -D flags from the build to correctly setup the environment for the > native code? I know this was a major deal when creating the > compile-commands.json for other native IDE integrations. I've heard some IDEs > just run the build once and inspect the command lines, but our default log > level won't show that. I'm not familiar with eclipse project files, but I > couldn't really see anything here that addressed this issue. Can you work > with the native code in a meaningful way without it? @erikj79 @magicus Seems like this is the best I can do for now, baseline support for Eclipse should now be present with these final touchups (Environment setup is not fully complete yet, but Eclipse is flexible enough that for most development work, this should not be too problematic). I do plan to refine support for it in the future, since there are still areas for improvement, but at present I think this should be decent enough for the time being - PR: https://git.openjdk.org/jdk/pull/10853
Re: RFR: 8294982: Implementation of Classfile API [v49]
On Tue, 7 Mar 2023 14:48:43 GMT, Adam Sotona wrote: >> src/java.base/share/classes/jdk/internal/classfile/impl/EntryMap.java line >> 194: >> >>> 192: return (int)s; >>> 193: } >>> 194: } >> >> newline! > > Here I'm also not sure I understand, the long line has bee wrapped. I mean here (and in the other) there seems to be a missing newline at the end of the file >> src/java.base/share/classes/jdk/internal/classfile/impl/TargetInfoImpl.java >> line 34: >> >>> 32: >>> 33: /** >>> 34: * TargetInfoImpl >> >> Does this javadoc add any value? Since this is implementation, we could also >> remove it. > > This is common practise across the whole implementation. Do you suggest to > remove all similar javadoc from all implementation classes? Well, if the javadoc simply states the name of the class it doesn't seem very useful. But I leave that decision to you. - PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v36]
On Tue, 7 Mar 2023 08:35:34 GMT, Adam Sotona wrote: >> I am unsure how you might use `BlockCodeBuilder`. If the current signature >> is not changed then `transformFromHandler` seems reasonable (since its the >> handler that pushes elements into its given builder). >> >> The other `transform` is implicitly "transform model". > > I'm sorry my answer was inaccurate, what I'm proposing is > `transformBlock(Consumer, CodeTransform)` or > `transformedBlock(Consumer, CodeTransform)`. > `BlockCodeBuilder` is just an extension of `CodeBuilder` with ability to > break block execution. > Block is a sub-unit of code, so I expect it would be a logical extension of > the actual `block(Consumer`. > I'll move this discussion to the mailing list to make common decision about > API change. > Thanks. This topic is now tracked as RFE - PR: https://git.openjdk.org/jdk/pull/10982
Integrated: 8303480: Miscellaneous fixes to mostly invisible doc comments
On Thu, 2 Mar 2023 12:03:44 GMT, Pavel Rappo wrote: > Please review this superficial documentation cleanup that was triggered by > unrelated analysis of doc comments in JDK API. > > The only effect that this multi-area PR has on the JDK API Documentation > (i.e. the observable effect on the generated HTML pages) can be summarized as > follows: > > > diff -ur build/macosx-aarch64/images/docs-before/api/serialized-form.html > build/macosx-aarch64/images/docs-after/api/serialized-form.html > --- build/macosx-aarch64/images/docs-before/api/serialized-form.html > 2023-03-02 11:47:44 > +++ build/macosx-aarch64/images/docs-after/api/serialized-form.html > 2023-03-02 11:48:45 > @@ -17084,7 +17084,7 @@ > throws href="java.base/java/io/IOException.html" title="class in > java.io">IOException, > ClassNotFoundException > readObject is called to restore the > state of the > - (@code BasicPermission} from a stream. > + BasicPermission from a stream. > > Parameters: > s - the ObjectInputStream from which data > is read > > Notes > - > > * I'm not an expert in any of the affected areas, except for jdk.javadoc, and > I was merely after misused tags. Because of that, I would appreciate reviews > from experts in other areas. > * I discovered many more issues than I included in this PR. The excluded > issues seem to occur in infrequently updated third-party code (e.g. > javax.xml), which I assume we shouldn't touch unless necessary. > * I will update copyright years after (and if) the fix had been approved, as > required. This pull request has now been integrated. Changeset: 45a616a8 Author:Pavel Rappo URL: https://git.openjdk.org/jdk/commit/45a616a891e4a4b0e77b1f2fa040522f4a99d172 Stats: 75 lines in 39 files changed: 0 ins; 0 del; 75 mod 8303480: Miscellaneous fixes to mostly invisible doc comments Reviewed-by: mullan, prr, cjplummer, aivanov, jjg, lancea, rriggs, ihse - PR: https://git.openjdk.org/jdk/pull/12826
Re: RFR: 8294982: Implementation of Classfile API [v53]
> This is root pull request with Classfile API implementation, tests and > benchmarks initial drop into JDK. > > Following pull requests consolidating JDK class files parsing, generating, > and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) > will chain to this one. > > Classfile API development is tracked at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch > > Development branch of consolidated JDK class files parsing, generating, and > transforming is at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch > > Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online > API > documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html) > is also available. > > Please take you time to review this non-trivial JDK addition. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: package jdk.internal.classfile.jdktypes moved to jdk.internal.classfile.java.lang.constant - Changes: - all: https://git.openjdk.org/jdk/pull/10982/files - new: https://git.openjdk.org/jdk/pull/10982/files/7a2b5cbe..a994c572 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10982=52 - incr: https://webrevs.openjdk.org/?repo=jdk=10982=51-52 Stats: 35 lines in 26 files changed: 0 ins; 0 del; 35 mod Patch: https://git.openjdk.org/jdk/pull/10982.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982 PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v52]
The message from this sender included one or more files which could not be scanned for virus detection; do not open these files unless you are certain of the sender's intent. -- > This is root pull request with Classfile API implementation, tests and > benchmarks initial drop into JDK. > > Following pull requests consolidating JDK class files parsing, generating, > and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) > will chain to this one. > > Classfile API development is tracked at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch > > Development branch of consolidated JDK class files parsing, generating, and > transforming is at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch > > Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online > API > documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html) > is also available. > > Please take you time to review this non-trivial JDK addition. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: fixed CodeRelabeler javadoc - Changes: - all: https://git.openjdk.org/jdk/pull/10982/files - new: https://git.openjdk.org/jdk/pull/10982/files/2b1bd7f8..7a2b5cbe Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10982=51 - incr: https://webrevs.openjdk.org/?repo=jdk=10982=50-51 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/10982.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982 PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v49]
On Tue, 7 Mar 2023 11:14:17 GMT, Maurizio Cimadamore
wrote:
>> Adam Sotona has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> snippets and tests synced with jdk.jfr class instrumentation source code
>
> src/java.base/share/classes/jdk/internal/classfile/components/CodeRelabeler.java
> line 44:
>
>> 42:
>> 43: /**
>> 44: * CodeRelabeler is a {@link jdk.internal.classfile.CodeTransform}
>> replacing all occurrences
>
> Nit - perhaps just use `A code relabeler is a ...` (instead of using the
> class name, which then would require another `{@code ... }`.
fixed, thanks.
> src/java.base/share/classes/jdk/internal/classfile/components/package-info.java
> line 114:
>
>> 112: * {@snippet lang="java" class="PackageSnippets"
>> region="classInstrumentation"}
>> 113: */
>> 114: package jdk.internal.classfile.components;
>
> watch out for newline
I'm not quite sure I understand what is wrong with this javadoc fragment.
Thanks.
> src/java.base/share/classes/jdk/internal/classfile/impl/EntryMap.java line
> 194:
>
>> 192: return (int)s;
>> 193: }
>> 194: }
>
> newline!
Here I'm also not sure I understand, the long line has bee wrapped.
> src/java.base/share/classes/jdk/internal/classfile/impl/TargetInfoImpl.java
> line 34:
>
>> 32:
>> 33: /**
>> 34: * TargetInfoImpl
>
> Does this javadoc add any value? Since this is implementation, we could also
> remove it.
This is common practise across the whole implementation. Do you suggest to
remove all similar javadoc from all implementation classes?
-
PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v15]
On Wed, 15 Feb 2023 14:24:18 GMT, Adam Sotona wrote:
>> src/java.base/share/classes/jdk/internal/classfile/instruction/CharacterRange.java
>> line 47:
>>
>>> 45: * {@return the start of the instruction range}
>>> 46: */
>>> 47: Label startScope();
>>
>> I noticed that this pattern of start/endScope is here, but also in
>> ExceptionCatch, LocalVariable and LocalVariableType. Consider using a common
>> interface for "instructions that are associated with a range".
>
> That is interesting RFE, thanks.
collected and tracked in the mailing list
-
PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v36]
On Fri, 3 Mar 2023 15:51:25 GMT, Adam Sotona wrote:
>> src/java.base/share/classes/jdk/internal/classfile/instruction/NewObjectInstruction.java
>> line 38:
>>
>>> 36: * of a {@link CodeModel}.
>>> 37: */
>>> 38: public sealed interface NewObjectInstruction extends Instruction
>>
>> Should we add a helper method on `CodeBuilder` that does the new + dup +
>> invoke special dance?
>
> That is great RFE for `CodeBuilder`, thanks.
collected and tracked in the mailing list
-
PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v43]
On Fri, 3 Mar 2023 22:48:23 GMT, Paul Sandoz wrote:
>> Adam Sotona has updated the pull request incrementally with three additional
>> commits since the last revision:
>>
>> - fixed AccessFlags javadoc
>> - TransformImpl.FakeXyzTransform renamed to UnresolvedXyzTransform
>> - removed obsolete generic parameter from AbstractDirectBuilder
>
> src/java.base/share/classes/jdk/internal/classfile/impl/EntryMap.java line
> 176:
>
>> 174: }
>> 175:
>> 176: public static long nextPowerOfTwo( long x ) {
>
> If you like you can change the implementation to be:
>
> x = -1 >>> Long.numberOfLeadingZeros(x - 1);
> return x + 1;
>
> See `ConcurrentHashMap`.
`Long:numberOfLeadingZeros` implementation uses more method calls, conditional
statements and binary operations. I would prefer to stick with the current
implementation for its speed and simplicity.
-
PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v36]
On Thu, 2 Mar 2023 22:05:24 GMT, Paul Sandoz wrote:
>> Adam Sotona has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> StackMapFrameInfo extracted to top level from StackMapTableAttribute
>
> src/java.base/share/classes/jdk/internal/classfile/impl/AbstractInstruction.java
> line 156:
>
>> 154: @Override
>> 155: public String toString() {
>> 156: return String.format("Store[OP=%s, slot=%d]",
>> this.opcode(), slot());
>
> A suggestion. I believe the `toString` output for bound and unbound
> instructions should be identical and it can composed solely from methods on
> the public instruction interface. There are some differences e.g. `Increment`
> and `Inc` for the unbound and bound increment instruction respectively.
>
> If those assumptions are correct i recommend placing a static `toString`
> method on all unbound instructions that also have bound instructions,
> accepting the public instruction interface as an argument. Then the
> overridden `Object::toString` method defers to those. Thereby there is no
> duplication.
> (Alas we cannot override `toString` on the instruction interface itself).
resolved using common static format Strings
-
PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v12]
On Mon, 6 Feb 2023 14:25:54 GMT, Adam Sotona wrote:
>> src/java.base/share/classes/jdk/internal/classfile/CodeBuilder.java line
>> 1371:
>>
>>> 1369: }
>>> 1370:
>>> 1371: default CodeBuilder tableswitch(Label defaultTarget,
>>> List cases) {
>>
>> `switch` seems the one instruction for which an high-level variant (like
>> `trying`) could be useful, as generating code for that can be quite painful.
>
> Nice RFE, thanks.
collected and tracked in the mailing list
-
PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v51]
> This is root pull request with Classfile API implementation, tests and > benchmarks initial drop into JDK. > > Following pull requests consolidating JDK class files parsing, generating, > and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) > will chain to this one. > > Classfile API development is tracked at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch > > Development branch of consolidated JDK class files parsing, generating, and > transforming is at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch > > Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online > API > documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html) > is also available. > > Please take you time to review this non-trivial JDK addition. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: Shared `toString` formats for bound and unbound instructions - Changes: - all: https://git.openjdk.org/jdk/pull/10982/files - new: https://git.openjdk.org/jdk/pull/10982/files/a87d0096..2b1bd7f8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10982=50 - incr: https://webrevs.openjdk.org/?repo=jdk=10982=49-50 Stats: 59 lines in 1 file changed: 23 ins; 0 del; 36 mod Patch: https://git.openjdk.org/jdk/pull/10982.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982 PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8302189: Mark assertion failures noreturn [v2]
On Sat, 4 Mar 2023 11:23:58 GMT, Kim Barrett wrote: >> Also 8302799: Refactor Debugging variable usage for noreturn crash reporting > > Kim Barrett has updated the pull request incrementally with one additional > commit since the last revision: > > make Debugging::_enabled a nesting counter Seems fine. I'll test drive with the debug commands next time I need to use them but this is similar to to the changes I have in my private repos. Thanks for keeping this functionality. src/hotspot/share/utilities/debug.hpp line 108: > 106: // because we need a fallback when we don't have any mechanism for > detecting > 107: // constant evaluation. > 108: #if defined(TARGET_COMPILER_gcc) || defined(TARGET_COMPILER_xlc) All this seems like it should go in COMPILER_HEADER(globalDefinitions.hpp) but since globalDefinitions.hpp includes debug.hpp, you can't do this. Can we file an RFE to clean this up (if possible)? - Marked as reviewed by coleenp (Reviewer). PR: https://git.openjdk.org/jdk/pull/12845
Re: RFR: 8294982: Implementation of Classfile API [v50]
> This is root pull request with Classfile API implementation, tests and > benchmarks initial drop into JDK. > > Following pull requests consolidating JDK class files parsing, generating, > and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) > will chain to this one. > > Classfile API development is tracked at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch > > Development branch of consolidated JDK class files parsing, generating, and > transforming is at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch > > Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online > API > documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html) > is also available. > > Please take you time to review this non-trivial JDK addition. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: generic implementation of ResolvedTransform - Changes: - all: https://git.openjdk.org/jdk/pull/10982/files - new: https://git.openjdk.org/jdk/pull/10982/files/6df1297e..a87d0096 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=10982=49 - incr: https://webrevs.openjdk.org/?repo=jdk=10982=48-49 Stats: 46 lines in 5 files changed: 1 ins; 18 del; 27 mod Patch: https://git.openjdk.org/jdk/pull/10982.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10982/head:pull/10982 PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v49]
On Mon, 6 Mar 2023 17:11:42 GMT, Adam Sotona wrote: >> This is root pull request with Classfile API implementation, tests and >> benchmarks initial drop into JDK. >> >> Following pull requests consolidating JDK class files parsing, generating, >> and transforming >> ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) will chain to >> this one. >> >> Classfile API development is tracked at: >> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch >> >> Development branch of consolidated JDK class files parsing, generating, and >> transforming is at: >> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch >> >> Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online >> API >> documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html) >> is also available. >> >> Please take you time to review this non-trivial JDK addition. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > snippets and tests synced with jdk.jfr class instrumentation source code Well done. The API does a great job at mastering the complexity (and, at time, ad-hocness) of the Java bytecode, and expose it in a way that is principled and useful to developers. src/java.base/share/classes/jdk/internal/classfile/impl/EntryMap.java line 194: > 192: return (int)s; > 193: } > 194: } newline! src/java.base/share/classes/jdk/internal/classfile/impl/TargetInfoImpl.java line 34: > 32: > 33: /** > 34: * TargetInfoImpl Does this javadoc add any value? Since this is implementation, we could also remove it. - Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v36]
On Thu, 2 Mar 2023 23:49:24 GMT, Paul Sandoz wrote:
>> Adam Sotona has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> StackMapFrameInfo extracted to top level from StackMapTableAttribute
>
> src/java.base/share/classes/jdk/internal/classfile/CodeTransform.java line 91:
>
>> 89: @Override
>> 90: default ResolvedTransform resolve(CodeBuilder builder) {
>> 91: return new TransformImpl.CodeTransformImpl(e -> accept(builder,
>> e),
>
> Make `CodeTransformImpl` generic in the class file element, rename to say
> `ResolvedTransformImpl` and reuse for the other `XxxTransform`?
Good idea, I'll fix it, thanks.
-
PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments [v2]
On Mon, 6 Mar 2023 20:22:48 GMT, Pavel Rappo wrote: >> Please review this superficial documentation cleanup that was triggered by >> unrelated analysis of doc comments in JDK API. >> >> The only effect that this multi-area PR has on the JDK API Documentation >> (i.e. the observable effect on the generated HTML pages) can be summarized >> as follows: >> >> >> diff -ur >> build/macosx-aarch64/images/docs-before/api/serialized-form.html >> build/macosx-aarch64/images/docs-after/api/serialized-form.html >> --- build/macosx-aarch64/images/docs-before/api/serialized-form.html >> 2023-03-02 11:47:44 >> +++ build/macosx-aarch64/images/docs-after/api/serialized-form.html >> 2023-03-02 11:48:45 >> @@ -17084,7 +17084,7 @@ >> throws > href="java.base/java/io/IOException.html" title="class in >> java.io">IOException, >> ClassNotFoundException >> readObject is called to restore the >> state of the >> - (@code BasicPermission} from a stream. >> + BasicPermission from a stream. >> >> Parameters: >> s - the ObjectInputStream from which data >> is read >> >> Notes >> - >> >> * I'm not an expert in any of the affected areas, except for jdk.javadoc, >> and I was merely after misused tags. Because of that, I would appreciate >> reviews from experts in other areas. >> * I discovered many more issues than I included in this PR. The excluded >> issues seem to occur in infrequently updated third-party code (e.g. >> javax.xml), which I assume we shouldn't touch unless necessary. >> * I will update copyright years after (and if) the fix had been approved, as >> required. > > Pavel Rappo has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains two additional > commits since the last revision: > > - Merge branch 'master' into 8303480 > - Initial commit Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.org/jdk/pull/12826
Re: RFR: 8294982: Implementation of Classfile API [v49]
The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.
--
On Mon, 6 Mar 2023 17:11:42 GMT, Adam Sotona wrote:
>> This is root pull request with Classfile API implementation, tests and
>> benchmarks initial drop into JDK.
>>
>> Following pull requests consolidating JDK class files parsing, generating,
>> and transforming
>> ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) will chain to
>> this one.
>>
>> Classfile API development is tracked at:
>> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch
>>
>> Development branch of consolidated JDK class files parsing, generating, and
>> transforming is at:
>> https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch
>>
>> Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online
>> API
>> documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/java.base/jdk/internal/classfile/package-summary.html)
>> is also available.
>>
>> Please take you time to review this non-trivial JDK addition.
>>
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional
> commit since the last revision:
>
> snippets and tests synced with jdk.jfr class instrumentation source code
src/java.base/share/classes/jdk/internal/classfile/components/CodeRelabeler.java
line 44:
> 42:
> 43: /**
> 44: * CodeRelabeler is a {@link jdk.internal.classfile.CodeTransform}
> replacing all occurrences
Nit - perhaps just use `A code relabeler is a ...` (instead of using the class
name, which then would require another `{@code ... }`.
src/java.base/share/classes/jdk/internal/classfile/components/package-info.java
line 114:
> 112: * {@snippet lang="java" class="PackageSnippets"
> region="classInstrumentation"}
> 113: */
> 114: package jdk.internal.classfile.components;
watch out for newline
-
PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8294982: Implementation of Classfile API [v36]
On Fri, 3 Mar 2023 16:33:49 GMT, Adam Sotona wrote:
>> src/java.base/share/classes/jdk/internal/classfile/impl/TransformImpl.java
>> line 63:
>>
>>> 61: private static final Runnable NOTHING = () -> { };
>>> 62:
>>> 63: interface FakeClassTransform extends ClassTransform {
>>
>> Rename to `UnresolvedXxxTransform`? I think that is a better name, since it
>> could appear in stack traces. Like with `XxxTransformImpl` it may be
>> possible to share across all implementations by mixing in?
>
> Renamed to `UnresolvedXyzTransform`, thanks.
> I'll consider conversion to generic form in a next step.
Unfortunately application of common generic `UnresolvedTransform` does
not work here.
-
PR: https://git.openjdk.org/jdk/pull/10982
Re: RFR: 8303691: Fedora based devkit build should load more packages from archive location
On Mon, 6 Mar 2023 20:28:22 GMT, Christoph Langer wrote: > When building Fedora based Linux devkits, rpm packages are downloaded from > locations at the Fedora project. > The latest/active versions reside under https://dl.fedoraproject.org while > older, archived versions live at https://archives.fedoraproject.org. > > It seems more releases have been archived in the meanwhile, so e.g. a build > based on Fedora 27, which is currently marked as default, would fail. > > I suggest to add a variable `LATEST_ARCHIVED_OS_VERSION` in the make file > that denotes the Fedora version up to which a download would be attempted > from archives. For later releases, the download is done from dl. Marked as reviewed by mbaesken (Reviewer). - PR: https://git.openjdk.org/jdk/pull/12888
Re: RFR: 8294982: Implementation of Classfile API [v36]
On Mon, 6 Mar 2023 17:45:27 GMT, Paul Sandoz wrote: >> I see what you mean. `transforming` was selected to represent the >> continuation of the code building process, similar to `trying` and >> `catching`. While `transform` may imply that the actual code builder gets >> transformed into something else. For example `MethodModel::transformCode` >> takes `CodeModel`, applies `CodeTransform` and after that the code of the >> actual method is finished. >> What about `transformBlock(BlockCodeBuilder, CodeTransform)`? > > I am unsure how you might use `BlockCodeBuilder`. If the current signature is > not changed then `transformFromHandler` seems reasonable (since its the > handler that pushes elements into its given builder). > > The other `transform` is implicitly "transform model". I'm sorry my answer was inaccurate, what I'm proposing is `transformBlock(Consumer, CodeTransform)` or `transformedBlock(Consumer, CodeTransform)`. `BlockCodeBuilder` is just an extension of `CodeBuilder` with ability to break block execution. Block is a sub-unit of code, so I expect it would be a logical extension of the actual `block(Consumer`. I'll move this discussion to the mailing list to make common decision about API change. Thanks. - PR: https://git.openjdk.org/jdk/pull/10982
