URGENT: Request for review: 7122222: GC log is limited to 2G for 32-bit

Tao Mao tao.mao at oracle.com
Wed Jun 5 00:06:44 UTC 2013


Thank you for review, Dan.

I'll try to answer as much as I can. Please see inline.

Thanks.
Tao

On 6/4/13 4:35 PM, Daniel D. Daugherty wrote:
> > http://cr.openjdk.java.net/~tamao/7122222/webrev.00/
>
> Tao,
>
> I think the lack of response to this review request is the absolutely
> strange nature of these changes. And I thought I put out some weird
> code reviews... :-)
>
> make/linux/makefiles/vm.make
>     Build ostream.o with _FILE_OFFSET_BITS==64 on Linux. Nothing
>     obvious in this webrev about what this will mean so I took a
>     look at src/share/vm/utilities/ostream.{c,h}pp and I see no
>     use of _FILE_OFFSET_BITS in either of those source files.
>
>     Must be in the source files somewhere, but I can't find any
>     use of _FILE_OFFSET_BITS in the entire hotspot source base.
>
> make/solaris/makefiles/vm.make
> Build ostream.o with _FILE_OFFSET_BITS==64 on Solaris.
>
>     OK, I looked for _FILE_OFFSET_BITS in /usr/include on my
>     Solaris box. Lots of references, but nothing that helps me
>     understand what you're doing here.
> src/os/solaris/vm/os_solaris.inline.hpp
>     The addition of _FILE_OFFSET_BITS==64 means that the
>     os::readdir() function will use the safer, multi-threaded
>     version of readdir_r(). Seems fine to me.
>
> Here's what I need to know:
>
> - what effect does _FILE_OFFSET_BITS have on building ostream.{c,h}pp?
_FILE_OFFSET_BITS is set to be picked by c++ compiler.

For why we need to set _FILE_OFFSET_BITS==64 in this case, please refer 
to the following document

This Sun White Paper 
(http://unix.business.utah.edu/doc/os/solaris/misc/largefiles.pdf) 
summarizes the usage of the flags on solaris (page "5-26"). And, it 
should apply to Linux the same way as was agreed across platforms 
(http://linuxmafia.com/faq/VALinux-kb/2gb-filesize-limit.html).

> - if ostream.o has one idea about the value of _FILE_OFFSET_BITS
>   what happens if another part of the VM has a different idea about
>   the value of _FILE_OFFSET_BITS?
_FILE_OFFSET_BITS is not set for other particular effects, but for 
extending the ability to deal with large files in ostream.{c,h}pp. So, 
if other files have a different idea about _FILE_OFFSET_BITS, they can't 
deal with large files. No more no less.

>
> I saw this in the post to the Runtime alias:
>
> > Included runtime dev to see whether they have some idea to handle
> > the compilation choices.
>
> And I still have no idea what you're asking here? What compilation
> choices? Are you asking about your Makefile changes? Are asking
> about defining _FILE_OFFSET_BITS for the entire build instead of
> just one object (ostream.o)? Are you worried that this VM is going
> to have mis-matched pieces and be unstable?
"Are asking about defining _FILE_OFFSET_BITS for the entire build 
instead of just one object (ostream.o)?" is my main question I 
originally tried to ask.

>
> So I reviewed it, but I definitely can't approve it without more
> info. I realize that you're up against the RDP2 limit, but this
> change has too many open questions (for now)...
>
> BTW, it is not at all clear whether Win32 will be able to write a 2GB+
> GC log or not. The conversation below didn't help me at all.
I used a jdk7 (just any) to successfully generate a log file larger than 
4GB. So, it shouldn't be a problem for Win32.
>
> Dan
>
>
>
> On 6/4/13 5:03 PM, Tao Mao wrote:
>> Since the changeset touched makefiles, I've included 
>> build-dev at openjdk.java.net .
>>
>> I need to push the hsx24 bug asap. Please review it.
>>
>> Thanks.
>> Tao
>>
>> On 6/4/13 2:37 PM, Tao Mao wrote:
>>> Hi all,
>>>
>>> Need reviews to catch RDP2.
>>>
>>> The current webrev is a working solution to all platforms, Linux, 
>>> Windows, and Solaris.
>>> http://cr.openjdk.java.net/~tamao/7122222/webrev.00/
>>>
>>> Thanks.
>>> Tao
>>>
>>> On 5/30/13 10:21 AM, Tao Mao wrote:
>>>> Included runtime dev to see whether they have some idea to handle 
>>>> the compilation choices.
>>>>
>>>> For now, it's been verified that the fix is functionally sufficient.
>>>>
>>>> Thanks.
>>>> Tao
>>>>
>>>> On 5/29/13 5:27 PM, Tao Mao wrote:
>>>>> Thank you, Mikael.
>>>>>
>>>>> Please see inline.
>>>>>
>>>>> Reviewers, please review it based on the following new observation.
>>>>>
>>>>> Tao
>>>>>
>>>>> On 5/27/13 2:05 AM, Mikael Gerdin wrote:
>>>>>> Tao,
>>>>>>
>>>>>> On 2013-05-25 02:19, Tao Mao wrote:
>>>>>>> 7ux bug
>>>>>>>
>>>>>>> webrev:
>>>>>>> http://cr.openjdk.java.net/~tamao/7122222/webrev.00/
>>>>>>>
>>>>>>> changeset:
>>>>>>> (1) make -D_FILE_OFFSET_BITS=64 only available to generating 
>>>>>>> ostream.o
>>>>>>>
>>>>>>> Why conservative rather than making -D_FILE_OFFSET_BITS=64 globally
>>>>>>> applicable?
>>>>>>>
>>>>>>> Global setting of -D_FILE_OFFSET_BITS=64 on linux works fine; 
>>>>>>> however,
>>>>>>> there are at least five code conflicts if introducing the flag 
>>>>>>> globally
>>>>>>> to Solaris.
>>>>>>>
>>>>>>> One was resolved as in os_solaris.inline.hpp, but the rest four 
>>>>>>> files
>>>>>>> had conflicts deep in c library. Even if they are excluded from 
>>>>>>> setting
>>>>>>> -D_FILE_OFFSET_BITS=64, the compiled VM is corrupted.
>>>>>>>
>>>>>>> (2) For now, no Windows solution.
>>>>>>> I haven't found any clean solution for solving this problem on 
>>>>>>> Windows.
>>>>>>
>>>>>> This seems like an insufficient fix if you can't make it work on 
>>>>>> all platforms. I tried building with "-D_LARGEFILE64_SOURCE 
>>>>>> -D_FILE_OFFSET_BITS=64" ons Solaris and hit an #error in libelf.h 
>>>>>> saying it wasn't supported so I understand your problem there.
>>>>> Yes, that's my grief :( you touched them, a bunch of them. That's 
>>>>> why I chose to apply the flag only to the files (ostream.cpp and 
>>>>> ostream.hpp) I want the effect.
>>>>>>
>>>>>> Instead I suggest that you use the compatibility API described in 
>>>>>> lf64(5) on Solaris. This API consists of fopen64, ftell64 and 
>>>>>> friends and is exposed when "-D_LARGEFILE64_SOURCE" is set.
>>>>>>
>>>>>> The same "-D_LARGEFILE64_SOURCE" is available on Linux and has 
>>>>>> the added advantage of not changing any existing symbols and 
>>>>>> therefore we can set the define for all files instead of just 
>>>>>> ostream
>>>>>>
>>>>>> This approach has the added advantage that it more closely 
>>>>>> resembles the changes which will be needed for Windows anyway. 
>>>>>> Those changes would consist of changing calls to ftell/fseek to 
>>>>>> 64-bit versions and changing fopen to fopen64 on Solaris/Linux.
>>>>> Both ways have pros and cons. The current implementation excludes 
>>>>> the usage of fopen64, providing portability (since there's no 
>>>>> fopen64 for Windows). Meanwhile, I understand your suggestion 
>>>>> provides other benefits.
>>>>>
>>>>> This Sun White Paper 
>>>>> (http://unix.business.utah.edu/doc/os/solaris/misc/largefiles.pdf) 
>>>>> summarizes the usage of the flags on solaris (Page 5-26). And, it 
>>>>> should apply to Linux the same way as was agreed across platforms.
>>>>>
>>>>>>
>>>>>> Since there is no fopen64 on Windows it seems that the default 
>>>>>> fopen already supports large files.
>>>>> I tested, and you are correct that the 32-bit VM on Windows can 
>>>>> write beyond 2GB (and beyond 4GB). Thank you, it's solved "half of 
>>>>> my problem" :)
>>>>>>
>>>>>> /Mikael
>>>>>>
>>>>>>>
>>>>>>> test:
>>>>>>> (1) Ability to write over 2g file for 32-bit builds were 
>>>>>>> verified on the
>>>>>>> following configurations.
>>>>>>>
>>>>>>> Linux * i586
>>>>>>> Solaris * i586
>>>>>>> Solaris * sparc
>>>>>>>
>>>>>>> (2) Need a JPRT test for sanity check.
>>>>>>
>



More information about the hotspot-gc-dev mailing list