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

4833719: (bf) Views of MappedByteBuffers are not MappedByteBuffers, and cannot be forced #2902

Closed
wants to merge 6 commits into from

Conversation

bplb
Copy link
Member

@bplb bplb commented Mar 9, 2021

Please consider this proposal to add covariant overrides to MappedByteBuffer for the methods compact(), duplicate(), slice(), and slice(int,int).

The methods in question are added as abstract specifications in MappedByteBuffer and their implementations in Direct-X-Buffer.java.template. In MappedByteBuffer the isSync() method is changed to have package access, and a final package scope method FileDescriptor fileDescriptor() is added to return the associated file descriptor. Specification verbiage is added to the new covariant overrides, and the specification of force() is enhanced slightly. (The unmapper() method offers an alternative way to obtain the file descriptor and sync mode without the need for package access fileDescriptor() and isSync() methods.)

In Direct-X-Buffer.java.template the constructor for duplicates and slices is modified to accept parameters for the file descriptor and sync mode for byte buffers. The uses of this constructor are correspondingly modified.

A test is added to exercise the new methods. Verifying that force() is actually doing anything is not verified by this test but was checked manually. The change passes all other existing tests in tiers 1-3.

Other methods for which it might be worth adding covariant overrides are the get() and put() methods which return a buffer, and, less interesting, the put$Type$() methods.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-4833719: (bf) Views of MappedByteBuffers are not MappedByteBuffers, and cannot be forced

Reviewers

Download

To checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/2902/head:pull/2902
$ git checkout pull/2902

To update a local copy of the PR:
$ git checkout pull/2902
$ git pull https://git.openjdk.java.net/jdk pull/2902/head

@bridgekeeper
Copy link

bridgekeeper bot commented Mar 9, 2021

👋 Welcome back bpb! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr Pull request is ready for review label Mar 9, 2021
@openjdk
Copy link

openjdk bot commented Mar 9, 2021

@bplb The following label will be automatically applied to this pull request:

  • nio

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the nio nio-dev@openjdk.org label Mar 9, 2021
@bplb
Copy link
Member Author

bplb commented Mar 9, 2021

/csr

@openjdk openjdk bot added the csr Pull request needs approved CSR before integration label Mar 9, 2021
@openjdk
Copy link

openjdk bot commented Mar 9, 2021

@bplb has indicated that a compatibility and specification (CSR) request is needed for this pull request.
@bplb please create a CSR request and add link to it in JDK-4833719. This pull request cannot be integrated until the CSR request is approved.

@mlbridge
Copy link

mlbridge bot commented Mar 9, 2021

Webrevs

Copy link
Contributor

@adinn adinn left a comment

Choose a reason for hiding this comment

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

This looks good.

@bplb
Copy link
Member Author

bplb commented Mar 15, 2021

@bplb
Copy link
Member Author

bplb commented Mar 18, 2021

Once the CSR is reviewed and appoved then this PR will be ready to integrate.

@openjdk openjdk bot removed the csr Pull request needs approved CSR before integration label Mar 25, 2021
@openjdk
Copy link

openjdk bot commented Mar 25, 2021

@bplb 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:

4833719: (bf) Views of MappedByteBuffers are not MappedByteBuffers, and cannot be forced

Reviewed-by: adinn

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 105 new commits pushed to the master branch:

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this 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 the ready Pull request is ready to be integrated label Mar 25, 2021
@bplb
Copy link
Member Author

bplb commented Mar 25, 2021

/integrate

@openjdk openjdk bot closed this Mar 25, 2021
@openjdk openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 25, 2021
@openjdk
Copy link

openjdk bot commented Mar 25, 2021

@bplb Since your change was applied there have been 118 commits pushed to the master branch:

  • 8307aa6: 8264165: jpackage BasicTest fails after JDK-8220266: Check help text contains plaform specific parameters
  • c037e1e: 8263454: com.apple.laf.AquaFileChooserUI ignores the result of String.trim()
  • a1e717f: 8264146: Make Mutex point to rather than embed _name
  • f69afba: 8263300: add HtmlId for the block containing a class's description.
  • d82464f: 8263528: Make static page ids safe from collision with language elements
  • d602ae0: 8263884: Clean up os::is_allocatable() across Posix platforms
  • a9d287a: 8260388: Listing (sub)packages at package level of API documentation
  • 8120064: 8263781: C2: Cannot hoist independent load above arraycopy
  • 9689863: 8262295: C2: Out-of-Bounds Array Load from Clone Source
  • a678a38: 8263743: redundant lock in SSLSocketImpl
  • ... and 108 more: https://git.openjdk.java.net/jdk/compare/6aa28b3bdb78eb041a963d659d61e1622bc43ef8...master

Your commit was automatically rebased without conflicts.

Pushed as commit b006f22.

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

@bplb bplb deleted the 4833719-MBB-force branch March 31, 2021 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrated Pull request has been integrated nio nio-dev@openjdk.org
Development

Successfully merging this pull request may close these issues.

5 participants