RFR: JDK-8218074 - Update Graal

Vladimir Kozlov vladimir.kozlov at oracle.com
Tue Mar 12 00:44:30 UTC 2019


I looked through all changes done in OpenJDK since last 'Graal Update'.

I found only few issues - see following. The rest changes in overwritten-diffs.txt are not overwritten, they are kept - 
we don't need to re-apply them.

-----------------------------------------------------------------------
New Igor's fix for 8196568 overwrote fix for 8217678 in StandardGraphBuilderPlugins.java file

http://hg.openjdk.java.net/jdk/jdk/rev/b693b0d2053d

Igor, please confirm that your changes also fix 8217678 and we don't need to apply it again.

-----------------------------------------------------------------------
Next changes were reverted: JDK-8217289

http://hg.openjdk.java.net/jdk/jdk/rev/0040f89feb78

Jesper, I would suggest to revert your changes to corresponding file:

  hg revert 
src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/BigIntegerIntrinsicsTest.java

I asked Patric to file PR for it month ago. Does anyone known if it was filed? I sent e-mail to him.

----------------------------------------------------------------------
Several files has incorrect Copyright years change from 2019 back to 2018. For example:

http://cr.openjdk.java.net/~jwilhelm/8218074/webrev.00/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.lir.amd64/src/org/graalvm/compiler/lir/amd64/AMD64ArrayIndexOfOp.java.udiff.html

Please Fix.

Doug, we need PR to update Copyright years in master repo so they match.

Thanks,
Vladimir

On 3/11/19 1:55 PM, jesper.wilhelmsson at oracle.com wrote:
>> On 11 Mar 2019, at 18:56, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>
>> I am fine doing small manual labor to find what is missing and what is overwritten. For that I need some information.
>>
>> First, how overwritten-diffs.txt was created? If it is created by using 'hg' to find all changes done in OpenJDK since last 'Update Graal' then I would understand why changes are in this file.
>>
>> Next, can you attach to bug report original patch created by updategraalinopenjdk before applying it to OpenJDK sources? Do we have it? This way I can compare overwritten-diffs.txt with it and find if there duplicate and I don't need to re-apply changes.
> 
> The mx script applies the patch automatically so I don't have it as a separate patch. But the only changes that my script does after the mx script has applied the patch is in the test directory, so the patch available in the webrev should be what the mx script created if you ignore anything in test.
> 
>> Some changes were pushed into OpenJDK because we did not want to wait a fix for months - last updated was done 3 months ago!!! If we do more frequent updates overwritten-diffs.txt will be empty or very small. Manual work should not take a lot of time in such case.
> 
> Yes, the intention is to do this more often, hopefully on a weekly basis going forward, and that should minimize the need to push to the OpenJDK as you say. I hope this "first" integration is a one-off in that respect. Obviously how often we can integrate depends on how much time the review takes ;-)
> /Jesper
> 
>>
>> Thanks,
>> Vladimir
>>
>> On 3/11/19 2:45 AM, Doug Simon wrote:
>>>> On 11 Mar 2019, at 01:56, jesper.wilhelmsson at oracle.com wrote:
>>>>
>>>> Hi Vladimir,
>>>>
>>>> I followed the given instructions and used mx to create the diff:
>>>>
>>>> mx --java-home=$JAVA_HOME updategraalinopenjdk cleanjdk/open 12
>>>>
>>>> I assume JDK-8215322 is in there because it was actually pushed to OpenJDK and now it was overwritten (with the same diff). It seems the mx script do not realize that it's the same cahange.
>>> Correct. To detect that we would have to add logic for diffs of diffs and the complexity didn’t seem worth it. I’d more more than happy to review a smart person’s attempt at adding it though ;-)
>>> -Doug
>>>> The diff and patch and all changes made was done by the new script, so no extra human labor has been put into it at this point. If a human is required to modify the overwritten-diff I suggest that is part of the review step (as now) and that a new patch is simply added to the subtask.
>>>>
>>>> I have removed JDK-8215322 from the diff now.
>>>>
>>>> Thanks,
>>>> /Jesper
>>>>
>>>>> On 9 Mar 2019, at 18:42, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>>
>>>>> Hi Jesper,
>>>>>
>>>>> Thank you for doing this.
>>>>>
>>>>> I looked on 8220387 to see what changes your webrev does not have and it is strange. You should not have such big difference. How you prepared overwritten-diffs.txt?
>>>>>
>>>>> For example, JAOTC changes "8215322: add @file support to jaotc" are in overwritten-diffs.txt. But changeset is listed in the 8218074 Graal changelog list [1].
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> [1] 8433917 Wed Dec 19 16:20:00 2018 -0800 Igor Ignatyev [GR-13142] Add at-file support to jaotc.
>>>>>
>>>>> On 3/9/19 12:46 AM, jesper.wilhelmsson at oracle.com wrote:
>>>>>> Hi,
>>>>>> Please review the patch to integrate the latest Graal changes into OpenJDK.
>>>>>> Graal tip to integrate: db79f81716886b7883370cd6ea1bbf5c42966fa5
>>>>>> JBS duplicates fixed by this integration:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8217161
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8218698
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8218859
>>>>>> JBS duplicates deferred to the next integration:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8214947
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8218074
>>>>>> Webrev: http://cr.openjdk.java.net/~jwilhelm/8218074/webrev.00/
>>>>>> This integration did overwrite changes already in place in OpenJDK. A subtask was created to hold the diff: https://bugs.openjdk.java.net/browse/JDK-8220387
>>>>>> Thanks,
>>>>>> /Jesper
>>>>
> 


More information about the hotspot-compiler-dev mailing list