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:25:30 UTC 2013 

Thank you for your explanation and a "OK", Dan.

Tao

On 6/4/13 5:21 PM, Daniel D. Daugherty wrote:
> OK, based on the largefiles.pdf write-up, your use of 
> _FILE_OFFSET_BITS=64
> is going to cause ostream.o to bind to various 64-bit versions of some
> functions. Based on my very hazy memory of my days in file system code,
> this binding is going to affect ostream.o only unless ostream.o is
> writing to some file with the foo64() function and some other code is
> also writing to the same file at the same time with foo(). However, if we
> have two different functions writing to the same open file at the same
> time, then we have bigger issues. :-)
>
> I'm good with these changes now. I agree that solving the problem of
> setting _FILE_OFFSET_BITS=64 for the entire VM build doesn't have to
> solved right now.
>
> Dan
>
>
> On 6/4/13 6:06 PM, Tao Mao wrote:
>> 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 build-dev mailing list