RFR (L/XXS) cleanup indent white space issues (8057107)

[Daniel D. Daugherty]

Fred,

Thanks for the fast review!

On 9/8/14 3:12 AM, Frederic Parain wrote:
> On 09/08/2014 10:38 AM, Frederic Parain wrote:
>> Dan,
>>
>> Thank you for cleaning this and the different webrevs.
>>
>> Only two minor comments:
>>
>> src/os/solaris/vm/os_solaris.cpp:1336
>>
>>     The comment looks less readable to me with the change
>>     (only a personal opinion).
>
> Replying to myself: this issue is addressed in your second
> changeset (8057109).

line_indent_filter.java passes through pre-processor lines without
modification so these two lines:

1340 #define SMALLINT 32   // libthread allocate for tsd_common is a 
version specific
1341 // small number - point is NO swap space available

were processed independently of each other. As you pointed out,
I manually fixed the lines in 8057109.


>
> Fred
>
>> src/os/windows/vm/os_windows.cpp:3682
>>
>>     Should the os::win32::_os_thread_limit name be aligned
>>     with other declarations (lines 3683-3685 and lines
>>     3690-3694)?

This would have to be a manual fix. The problem with
format alignments like this is figuring out the intent
when the surrounding lines are inconsistent due to
evolution of and addition to the code.

3679 int    os::win32::_vm_page_size       = 0;
3680 int    os::win32::_vm_allocation_granularity = 0;
3681 int    os::win32::_processor_type     = 0;
3682 // Processor level is not available on non-NT systems, use 
vm_version instead
3683 int    os::win32::_processor_level    = 0;
3684 julong os::win32::_physical_memory    = 0;
3685 size_t os::win32::_default_stack_size = 0;
3686
3687 intx os::win32::_os_thread_limit    = 0;
3688 volatile intx os::win32::_os_thread_count    = 0;
3689
3690 bool   os::win32::_is_nt              = false;
3691 bool   os::win32::_is_windows_2003    = false;
3692 bool   os::win32::_is_windows_server  = false;
3693
3694 bool   os::win32::_has_performance_count = 0;

Line 3688 used to be aligned at the '=' until the 'volatile'
was added. Lines 3680 and 3694 were added, but the surrounding
lines were not adjusted to new location of the '='.

I'll take care of these in the manual bug (8057109).


>>
>> Otherwise looks good to me.

Thanks!

Dan


>>
>> Fred
>>
>> On 09/07/2014 09:13 PM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> I have the fix for the following bug ready for JDK9 RT_Baseline:
>>>
>>>      JDK-8057107 cleanup indent white space issues prior to Contended
>>>                  Locking reorder and cache line bucket
>>>      https://bugs.openjdk.java.net/browse/JDK-8057107
>>>
>>> This is both a (L)arge and e(X)tra e(X)tra (S)mall review! Here is the
>>> URL for the (L)arge webrev:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8057107-webrev/0-jdk9-hs-rt/
>>>
>>> Since these are white space fixes, I recommend the udiff links instead
>>> of the frames links. Of course, some folks prefer the patch file itself
>>> for white space fixes.
>>>
>>> Here is the URL for the e(X)tra e(X)tra (S)mall webrev which was
>>> generated
>>> with a version of webrev that ignores all whitespace changes:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8057107-webrev/0-jdk9-hs-rt-no_ws/
>>>
>>> Again, I recommend the udiff links since that's the fastest way to see
>>> that webrev_no_ws found no non-white space changes. The patch file for
>>> this webrev shows all the white space changes.
>>>
>>> These indent white space cleanups were done in preparation for the
>>> reorder
>>> and cache line bucket from the Contended Locking project; these changes
>>> were made by my do_all_line_filters.ksh script. The script does not fix
>>> everything in these files that is not in sync with HotSpot style, 
>>> but it
>>> does fix a large portion of them.
>>>
>>> Thanks, in advance, for any comments, questions or suggestions.
>>>
>>> At the end of the review cycle for this fix, I will attach the 
>>> version of
>>> the do_all_line_filters.ksh script and the line_indent_filter.java used
>>> to do the work to JDK-8057107.
>>>
>>> Dan
>>>
>>>
>>
>