[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-graal-dev
Subject:    Re: RFR: 8312235: [JVMCI] ConstantPool should not force eager resolution [v4]
From:       Doug Simon <dnsimon () openjdk ! org>
Date:       2023-07-27 8:42:01
Message-ID: 3zHPbZa6SeEVeNdxtMJWNuLUr_ntmx-0ds-KUOkix8w=.605bf7a8-78ec-4dca-b078-b0caf5d63d04 () github ! com
[Download RAW message or body]

On Wed, 26 Jul 2023 16:11:11 GMT, Doug Simon <dnsimon@openjdk.org> wrote:

> > The existing `jdk.vm.ci.meta.ConstantPool.lookupConstant(int cpi)` method forces \
> > eager resolving of constants. For `DynamicConstant`, `MethodHandle` and \
> > `MethodType`, this can mean invoking bootstrap methods, something that should not \
> > be done during JIT compilation. To avoid this, this PR adds the following to \
> > `jdk.vm.ci.meta.ConstantPool`: 
> > 
> > /**
> > * Looks up a constant at the specified index.
> > *
> > * If {@code resolve == false} and the denoted constant is of type
> > * {@code JVM_CONSTANT_Dynamic}, {@code JVM_CONSTANT_MethodHandle} or
> > * {@code JVM_CONSTANT_MethodType} and it's not yet resolved then
> > * {@code null} is returned.
> > *
> > * @param cpi the constant pool index
> > * @return the {@code Constant} or {@code JavaType} instance representing the \
> >                 constant pool
> > *         entry
> > */
> > Object lookupConstant(int cpi, boolean resolve);
> > 
> > 
> > Likewise, `jdk.vm.ci.meta.ConstantPool.lookupBootstrapMethodInvocation` has been \
> > fixed to no longer invoke the associated bootstrap method. 
> > ---------
> > ### Progress
> > - [x] Change must be properly reviewed (1 review required, with at least 1 \
> >                 [Reviewer](https://openjdk.org/bylaws#reviewer))
> > - [x] Change must not contain extraneous whitespace
> > - [x] Commit message must refer to an issue
> > 
> > 
> > 
> > ### Reviewing
> > <details><summary>Using <code>git</code></summary>
> > 
> > Checkout this PR locally: \
> > `$ git fetch https://git.openjdk.org/jdk.git pull/14927/head:pull/14927` \
> > `$ git checkout pull/14927`
> > 
> > Update a local copy of the PR: \
> > `$ git checkout pull/14927` \
> > `$ git pull https://git.openjdk.org/jdk.git pull/14927/head`
> > 
> > </details>
> > <details><summary>Using Skara CLI tools</summary>
> > 
> > Checkout this PR locally: \
> > `$ git pr checkout 14927`
> > 
> > View PR using the GUI difftool: \
> > `$ git pr show -t 14927`
> > 
> > </details>
> > <details><summary>Using diff file</summary>
> > 
> > Download this PR as a diff file: \
> > <a href="https://git.openjdk.org/jdk/pull/14927.diff">https://git.openjdk.org/jdk/pull/14927.diff</a>
> >  
> > </details>
> 
> Doug Simon has updated the pull request incrementally with one additional commit \
> since the last revision: 
> renamed index to cp_index

Thanks for the reviews!

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14927#issuecomment-1653163435


[prev in list] [next in list] [prev in thread] [next in thread]