6348631 - request for review (updated)

Ivan Krylov Ivan.Krylov at oracle.com
Tue Nov 30 16:23:52 PST 2010


  I will file an RFE.
I did not bring in those includes - there were originally in hpi.hpp
And to move those inlined methods from .hpp to .cpp as not inlined .. I 
would have had to prove no performance hit.

Perhaps, the best time to do it would be when(if) we get to solaris + 
linux -> posix-os refactoring.
Thanks,
Ivan


On 12/1/10 2:58 AM, Paul Hohensee wrote:
> Coleen's right, if we can avoid #include'ing system include files in 
> header
> files, we should.  I grep'ed for #include + hpp + '<' and got 15 
> header files
> (outside of hpi, zero and shark) that do so.  Maybe file an rfe to fix 
> it later?
>
> Paul
>
> On 11/30/10 5:52 PM, coleen phillimore wrote:
>> ok.
>> thanks,
>> Coleen
>>
>> On 11/30/10 16:23, Ivan Krylov wrote:
>>> Coleen,
>>>
>>> I inlined those functions because they were inlined in the hpi 
>>> files. I tried to stick to the original code where possible.
>>>
>>> Thanks,
>>> Ivan
>>>
>>> On 11/30/10 9:28 PM, coleen phillimore wrote:
>>>>
>>>> Ivan,
>>>> This looks good to me.  I have some concerns with the inlined 
>>>> socket functions in:
>>>>
>>>> http://cr.openjdk.java.net/~ikrylov/6348631.v3/src/os/linux/vm/os_linux.inline.hpp.cdiff.html
>>>>
>>>> as it causes the .hpp file to import linux system specific hpp 
>>>> files which might cause namespace issues down the line.  I'd rather 
>>>> these sorts of includes be in a .cpp file.  I don't know if they 
>>>> were inlined for performance reasons though, but it seems unlikely 
>>>> to have any benefit.
>>>>
>>>> Everything else looks great - I like how it removes the #include 
>>>> hpi.hpp in files that don't even use hpi.
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 11/30/10 06:54, Ivan Krylov wrote:
>>>>>
>>>>> In light of the changes related to the includeDB fix and a few 
>>>>> comments on the previous webrev I am sending a new
>>>>> request for review
>>>>> Webrev: http://cr.openjdk.java.net/~ikrylov/6348631.v3/
>>>>>
>>>>> On 18.11.2010 10:07, Ivan Krylov wrote:
>>>>>> With this fix we are removing the use of the HPI (Host Portable 
>>>>>> interface) library from jvm.
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~ikrylov/6348631/
>>>>>>
>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-dev/attachments/20101201/96100e03/attachment-0001.html 


More information about the hotspot-dev mailing list