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:34:44 UTC 2013


Thank you for reviewing, Tim.

Tao

On 6/4/13 5:30 PM, Tim Bell wrote:
> I am OK with the Makefile changes.
>
> Dan - Thanks for looking after the deeper questions.
>
> Tim
>
> On 06/ 4/13 05:25 PM, Tao Mao wrote:
>> 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 hotspot-gc-dev mailing list