JEP 175 - Review comments

Steve Poole spoole at linux.vnet.ibm.com
Thu Jun 13 02:03:22 PDT 2013 

On 12 Jun 2013, at 15:56, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> On 6/11/13 11:28 PM, Steve Poole wrote:
>> When you say 'closed code' what are we talking about here?  What functionality?
> 
> Oracle code for ppc and arm. I am verifying that we don't have conflicts with ppc-aix merge.
> 
>> Just to be clear - fixing up the open code to suit Oracles closed code is not our responsibility and should not be a gate to the merging in of the ppc-aix project.
> 
> Not true, your changes in shared code affects our code so you pushing your problems on us.
> You should avoid shared code modification which affects our current sources and builds. At least in your initial big changesets.
> 
That's correct - we will work to ensure that the OpenJDK platforms are not broken by these contributions.  However that does not include Oracles closed platforms.
We understand that our contributions should not disadvantage the existing OpenJDK platforms on  Solaris, Linux, Windows and Mac and that our contributions should be 
reasonable in quality and approach. 

 No one ever said that we would be held to account for impact to Oracles closed code.   In fact  Volker and I spoke to Mark Reinhold on this topic (concerning Oracles PPC code)
as FOSDEM and I'm pretty sure we agreed that dealing with any merge impact on Oracles implementation was Oracles problem.

I'm not trying to be contentious - I just want to be sure we are all operating from the same principles.  I'm sure that we'd all be open to changes to our code to improve portability
of OpenJDK but not as a gating factor in merging the OpenJDK PPCAIX port into the OpenJDK main repositories.



> As I said, I am fine to do reasonable closed code adjustment but as separate small changeset later. For example, to convert adapter_code_size variable to function as in your changes.
> 
> Regards,
> Vladimir
> 
>> 
>>  On 11 Jun 2013, at 17:36, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>> 
>>> Forgot to include ppc-aix mail alias. Which Hotspot mail alias we will use for reviews?
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>> 
>>> -------- Original Message --------
>>> Subject: Re: JEP 175 - Review comments
>>> Date: Tue, 11 Jun 2013 09:33:59 -0700
>>> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
>>> CC: Volker Simonis <volker.simonis at gmail.com>,  Azeem Jiva <azeem.jiva at oracle.com>, Chris Plummer <chris.plummer at oracle.com>
>>> 
>>> Here is result of my first attempt to build/test ppc changes together
>>> with our closed sources.
>>> 
>>> Small problems:
>>> 
>>> src/share/vm/memory/allocation.hpp:209: Trailing whitespace
>>> src/share/vm/memory/allocation.inline.hpp:121: Trailing whitespace
>>> src/share/vm/opto/escape.cpp:2207: Trailing whitespace
>>> src/share/vm/memory/allocation.hpp:212: Trailing whitespace
>>> src/share/vm/opto/escape.cpp:2214: Trailing whitespace
>>> 
>>> Build on MacOS:
>>> 
>>> src/share/vm/opto/callGenerator.cpp: In member function 'virtual
>>> JVMState* VirtualCallGenerator::generate(JVMState*)':
>>> src/share/vm/opto/callGenerator.cpp:204: error:
>>> 'zero_page_read_protected' is not a member of 'os'
>>> 
>>> agent/src/os/bsd/MacosxDebuggerLocal.m:54:2: error: #error UNSUPPORTED_ARCH
>>> agent/src/os/bsd/MacosxDebuggerLocal.m:167:4: error: #error UNSUPPORTED_ARCH
>>> agent/src/os/bsd/MacosxDebuggerLocal.m:591:2: error: #error UNSUPPORTED_ARCH
>>> 
>>> 
>>> We have several conflict with closed sources builds:
>>> 
>>> src/share/vm/utilities/elfSymbolTable.cpp: In member function 'bool
>>> ElfSymbolTable::lookup(unsigned char*, int*, int*, int*)':
>>> src/share/vm/utilities/elfSymbolTable.cpp:97: error: comparison between
>>> signed and unsigned integer expressions
>>> src/share/vm/utilities/elfSymbolTable.cpp:124: error: comparison between
>>> signed and unsigned integer expressions
>>> 
>>> error: no 'oopDesc** frame::interpreter_frame_mirror_addr() const'
>>> member function declared in class 'frame'
>>> 
>>> error: no matching function for call to
>>> 'SharedRuntime::c_calling_convention(BasicType*&, VMRegPair*&, uint&)'
>>> 
>>> I think it is due to changes like next:
>>> 
>>> -#ifdef PPC
>>> +#ifdef PPC32
>>>   oop* interpreter_frame_mirror_addr() const;
>>> 
>>> 
>>> Next is easy fix in our closed sources but it requires efforts from our
>>> side:
>>> 
>>> src/share/vm/prims/methodHandles.cpp: In static member function 'static
>>> void MethodHandles::generate_adapters()':
>>> src/share/vm/prims/methodHandles.cpp:68: error: 'adapter_code_size'
>>> cannot be used as a function
>>> src/share/vm/prims/methodHandles.cpp:70: error: 'adapter_code_size'
>>> cannot be used as a function
>>> 
>>> 
>>> I would suggest to do such shared changes which affects different builds
>>> later after initial push.
>>> 
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>> On 6/7/13 3:20 AM, Lindenmaier, Goetz wrote:
>>>> Hi Vladimir,
>>>> 
>>>> I updated the repo to jdk8-b92.  Our nightly tests built and
>>>> tested it successfully.  In case you experience any problems
>>>> please tell me the details so I can fix them.
>>>> 
>>>> Best regards,
>>>>   Goetz.
>>>> 
>>>> http://cr.openjdk.java.net/~simonis/ppc-aix-port/index.html
>>>> 
>>>> 
>>>> -----Original Message-----
>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>> Sent: Dienstag, 4. Juni 2013 20:58
>>>> To: Simonis, Volker
>>>> Cc: Iris Clark; Wintergerst, Michael; Lindenmaier, Goetz; Bernard Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com; Tim Ellison; Alan Bateman
>>>> Subject: Re: JEP 175 - Review comments
>>>> 
>>>> Volker,
>>>> 
>>>> Can you or someone update
>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot to match latest
>>>> sources in http://hg.openjdk.java.net/jdk8/jdk8/hotspot?
>>>> I just tried to merge them and build Hotspot on x86 without success.
>>>> 
>>>> Thanks,
>>>> Vladimir
>>>> 
>>>> On 6/4/13 7:04 AM, Simonis, Volker wrote:
>>>>> 
>>>>> We intentionally used 'porters-dev' rather than'ppc-aix-port-dev' in the
>>>>> beginning to address a broader audience for the initial discussions.
>>>>> 
>>>>> But I'm happy to change that back to 'ppc-aix-port-dev' now.
>>>>> 
>>>>> Regards,
>>>>> Volker
>>>>> 
>>>>> 
>>>>> ------------------------------------------------------------------------
>>>>> *From:* Iris Clark [iris.clark at oracle.com]
>>>>> *Sent:* Tuesday, June 04, 2013 3:50 PM
>>>>> *To:* Simonis, Volker; Wintergerst, Michael; Lindenmaier, Goetz; Bernard
>>>>> Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael
>>>>> Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com; Tim Ellison;
>>>>> iris.clark at oracle.com
>>>>> *Cc:* Alan Bateman; Vladimir Kozlov
>>>>> *Subject:* RE: JEP 175 - Review comments
>>>>> 
>>>>> Hi, Volker.
>>>>> 
>>>>> Sorry about this, one more thing about the JEP...
>>>>> 
>>>>> I think that the "Discussion" list probably needs to be updated to be
>>>>> your Project's mailing list (ppc-aix-port-dev).  Right now it's listed
>>>>> as porters-dev.
>>>>> 
>>>>> Thanks,
>>>>> 
>>>>> iris
>>>>> 
>>>>> *From:*Simonis, Volker [mailto:volker.simonis at sap.com]
>>>>> *Sent:* Tuesday, June 04, 2013 12:12 AM
>>>>> *To:* Iris Clark; Wintergerst, Michael; Lindenmaier, Goetz; Bernard
>>>>> Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael
>>>>> Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com; Tim Ellison
>>>>> *Cc:* Alan Bateman; Vladimir Kozlov
>>>>> *Subject:* RE: JEP 175 - Review comments
>>>>> 
>>>>> Hi Iris,
>>>>> 
>>>>> you're right, the title is too clumsy - I just couldn't come up with
>>>>> something better yesterday in the evening.
>>>>> 
>>>>> "PowerPC/AIX Port" sounds good to me. If nobody complains, I'll take it.
>>>>> 
>>>>> Thanks,
>>>>> Volker
>>>>> 
>>>>> ------------------------------------------------------------------------
>>>>> 
>>>>> *From:*Iris Clark [iris.clark at oracle.com]
>>>>> *Sent:* Tuesday, June 04, 2013 2:57 AM
>>>>> *To:* Simonis, Volker; Wintergerst, Michael; Lindenmaier, Goetz; Bernard
>>>>> Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael
>>>>> Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com
>>>>> <mailto:luchsh at cn.ibm.com>; Tim Ellison
>>>>> *Cc:* Alan Bateman; Vladimir Kozlov; iris.clark at oracle.com
>>>>> <mailto:iris.clark at oracle.com>
>>>>> *Subject:* RE: JEP 175 - Review comments
>>>>> 
>>>>> Hi, Volker.
>>>>> 
>>>>> I've just updated the JEP according to your  suggestions.
>>>>> 
>>>>> I'm not a Reviewer however, I think that the phrase "OpenJDK master
>>>>> repositories" in the revised title is not ideal:
>>>>> 
>>>>> --- a/jep-175.md     Mon May 27 23:22:51 2013 +0400
>>>>> 
>>>>> +++ b/jep-175.md     Mon Jun 03 18:51:18 2013 +0200
>>>>> 
>>>>> @@ -1,5 +1,5 @@
>>>>> 
>>>>> JEP: 175
>>>>> 
>>>>> -Title: Integrate PowerPC/AIX Port into JDK 8
>>>>> 
>>>>> +Title: Integrate the PowerPC/AIX Port into the OpenJDK master repositories
>>>>> 
>>>>> Author: Volker Simonis
>>>>> 
>>>>> Organization: SAP AG
>>>>> 
>>>>> Created: 2013/1/11
>>>>> 
>>>>> Thinking out loud, what about just "PowerPC/AIX Port"?  JEPs are all
>>>>> about adding features to JDK Release Projects.  There are lots of
>>>>> examples of JEPs [1] which don't begin with verbs, e.g. "133: Unicode
>>>>> 6.2", "148: Small VM", "172: DocLint", etc.  The JEP itself contains
>>>>> additional details.
>>>>> 
>>>>> Perhaps others have suggestions?
>>>>> 
>>>>> Am I right with my assumption that the targeted release will still be
>>>>> JDK 8 (or 9/8u respectively) but that the targeted release will be set
>>>>> in the "Release" header field of the JEP by the OpenJDK Lead (as
>>>>> specified in the JEP specification)?
>>>>> 
>>>>> Yes.  Once that field is populated, the value will appear in the JEP
>>>>> index [1], see the third column.
>>>>> 
>>>>> Thanks,
>>>>> 
>>>>> Iris
>>>>> 
>>>>> [1]: http://openjdk.java.net/jeps/0
>>>>> 
>>>>> *From:*Simonis, Volker [mailto:volker.simonis at sap.com]
>>>>> *Sent:* Monday, June 03, 2013 10:10 AM
>>>>> *To:* Iris Clark; Wintergerst, Michael; Lindenmaier, Goetz; Bernard
>>>>> Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael
>>>>> Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com
>>>>> <mailto:luchsh at cn.ibm.com>; Tim Ellison
>>>>> *Cc:* Alan Bateman; Vladimir Kozlov
>>>>> *Subject:* RE: JEP 175 - Review comments
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> I've just updated the JEP according to your  suggestions. Please find
>>>>> the new version attached to this mail (I haven't checked the new version
>>>>> in until now to give everybody a chance to comment on the changes).
>>>>> 
>>>>> Am I right with my assumption that the targeted release will still be
>>>>> JDK 8 (or 9/8u respectively) but that the targeted release will be set
>>>>> in the "Release" header field of the JEP by the OpenJDK Lead (as
>>>>> specified in the JEP specification)?
>>>>> 
>>>>> In addition to the changes proposed by you I've added the contents of
>>>>> the "Approach" section from Azeems "PPCAIX plan" to the "Description"
>>>>> section of the JEP. I've also added links to the new "PowerPC/AIX Port
>>>>> Integration Plan" [2] of our "PowerPC/AIX Port OpenJDK Wiki Space" [3]
>>>>> to the JEP.
>>>>> 
>>>>> The "PowerPC/AIX Port Integration Plan" in the Wiki is intended to hold
>>>>> Azeems "PPCAIX plan" document.
>>>>> 
>>>>> @Iris: could you please somehow arrange to give Azeem editing rights to
>>>>> that page?
>>>>> 
>>>>> @Azeem: could you please be so kind to past the contents of the "PPCAIX
>>>>> plan" into that page (once you have the appropriate rights)? I saw that
>>>>> the document is created from an Atlassian Confluence Wiki anyway and in
>>>>> my unlimited naivety I imagine this could be a simple copy-and-paste
>>>>> operation:) If that doesn't work so easily, please let me know how I
>>>>> could help.
>>>>> 
>>>>> If there are no objections I plan to checkin the new version of the JEP
>>>>> tomorrow after our telephone call.
>>>>> 
>>>>> Thank you and best regards,
>>>>> Volker
>>>>> 
>>>>> [2]: https://wiki.openjdk.java.net/pages/viewpage.action?pageId=13729959
>>>>> [3]: https://wiki.openjdk.java.net/display/PPCAIXPort
>>>>> 
>>>>> ------------------------------------------------------------------------
>>>>> 
>>>>> *From:*Iris Clark [iris.clark at oracle.com]
>>>>> *Sent:* Friday, May 31, 2013 8:48 PM
>>>>> *To:* Wintergerst, Michael; Simonis, Volker; Lindenmaier, Goetz; Bernard
>>>>> Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael
>>>>> Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com
>>>>> <mailto:luchsh at cn.ibm.com>; Tim Ellison
>>>>> *Cc:* iris.clark at oracle.com <mailto:iris.clark at oracle.com>; Alan
>>>>> Bateman; Vladimir Kozlov
>>>>> *Subject:* JEP 175 - Review comments
>>>>> 
>>>>> Hi, Volker.
>>>>> 
>>>>> JEP 175: Integrate PowerPC/AIX Port into JDK 8
>>>>> 
>>>>> http://openjdk.java.net/jeps/175
>>>>> 
>>>>> We're actively working to get your JEP to Funded.   We had a few comments:
>>>>> 
>>>>> -Recommend that "8" be removed from the JEP title, etc.
>>>>> 
>>>>> -Recommend that the first Motivation bullet clearly indicate that it is
>>>>> only covering PPC/AIX.
>>>>> 
>>>>> -Recommend that the second Motivation bullet be modified to make it
>>>>> clear that it applies to Hotspot only.
>>>>> 
>>>>> (Instructions for editing the JEP may be found in the "Mechanics"
>>>>> section at the bottom of JEP 1 [1].)
>>>>> 
>>>>> Vladimir Kozlov (VM) and Alan Bateman (Core Libraries) are lined up to
>>>>> be the JEP's reviewers.  Once they're satisfied with your
>>>>> changes/feedback they'll add themselves to the JEP's "Reviewed-by" line.
>>>>> 
>>>>> Thanks,
>>>>> 
>>>>> Iris Clark
>>>>> 
>>>>> [1]: http://openjdk.java.net/jeps/1
>>>>> 
>>> 
>>> 
>> 
>

More information about the ppc-aix-port-dev mailing list