URGENT: Request for review: 7122222: GC log is limited to 2G for 32-bit
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Jun 4 23:35:11 UTC 2013
> 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?
- 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?
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?
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.
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