RFR (L/S) manual cleanup of white space issues (8057109)

[Coleen Phillimore]

Hi Dan,

In general, doing things like whitespace cleanups makes merging new 
fixes that need to be backported for some more difficult, but it appears 
that the cleanups that you've chosen not interfere with backports of 
components other than the locking subsystems.  So I like this change.   
The only exception to these changes that I think might interfere with 
other changes to other components:

http://cr.openjdk.java.net/~dcubed/8057109-webrev/1-jdk9-hs-rt/src/share/vm/runtime/thread.cpp.udiff.html

-    jint res = Atomic::cmpxchg(strong_roots_parity, &_oops_do_parity, thread_parity);
+    jint res = Atomic::cmpxchg(strong_roots_parity, &_oops_do_parity,
+                               thread_parity);


-void JavaThread::oops_do(OopClosure* f, CLDClosure* cld_f, CodeBlobClosure* cf) {
+void JavaThread::oops_do(OopClosure* f, CLDClosure* cld_f,
+                         CodeBlobClosure* cf) {


-void CompilerThread::oops_do(OopClosure* f, CLDClosure* cld_f, CodeBlobClosure* cf) {
+void CompilerThread::oops_do(OopClosure* f, CLDClosure* cld_f,
+                             CodeBlobClosure* cf) {


-void Threads::possibly_parallel_oops_do(OopClosure* f, CLDClosure* cld_f, CodeBlobClosure* cf) {
+void Threads::possibly_parallel_oops_do(OopClosure* f, CLDClosure* cld_f,
+                                        CodeBlobClosure* cf) {

-  assert(!is_par ||
-         (SharedHeap::heap()->n_par_threads() ==
-         SharedHeap::heap()->workers()->active_workers()), "Mismatch");
+  assert(!is_par || (SharedHeap::heap()->n_par_threads() ==
+                     SharedHeap::heap()->workers()->active_workers()),
+                     "Mismatch");


These might interfere with GC changes and these are really only to fit 
the line into 80 columns which, even though it's mine and your 
preference, isn't part of the coding standard.

So I think these 5 should be reverted.

The rest look great.

Coleen

On 9/8/14, 6:01 PM, Daniel D. Daugherty wrote:
> Updated after Fred's code review:
>
> Here is the URL for the (L)arge webrev:
>
> http://cr.openjdk.java.net/~dcubed/8057109-webrev/1-jdk9-hs-rt/
>
> Here is the URL for the (S)mall webrev which was generated with a version
> of webrev that ignores all whitespace changes:
>
> http://cr.openjdk.java.net/~dcubed/8057109-webrev/1-jdk9-hs-rt-no_ws/
>
> Dan
>
>
> On 9/7/14 1:13 PM, Daniel D. Daugherty wrote:
>> Greetings,
>>
>> I have the fix for the following bug ready for JDK9 RT_Baseline:
>>
>>     JDK-8057109 manual cleanup of white space issues prior to Contended
>>                 Locking reorder and cache line bucket
>>     https://bugs.openjdk.java.net/browse/JDK-8057109
>>
>> This is both a (L)arge and a (S)mall review! Here is the URL for the
>> (L)arge webrev:
>>
>> http://cr.openjdk.java.net/~dcubed/8057109-webrev/0-jdk9-hs-rt/
>>
>> Since these are mostly 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 (S)mall webrev which was generated with a 
>> version
>> of webrev that ignores all whitespace changes:
>>
>> http://cr.openjdk.java.net/~dcubed/8057109-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 minimal non-white space changes. The patch file
>> for this webrev shows all the changes.
>>
>> These manual changes were motivated by various review comments made for
>> earlier fixes in the Contended Locking project. During those reviews, I
>> deferred making some of the requested format changes to this round of
>> manual changes. In most cases, the reviewers posted comments with one
>> or two examples of things they did not like and I've swept all 17 files
>> that we've targeted for cleanup in the Contended Locking project looking
>> for those style problems and fixing them. I've made a total of 12 passes
>> over the 17 files, but I cannot say that I've caught everything. Most
>> stuff that annoys people is fixed, but it's very difficult to catch and
>> fix them all without a real parser based solution.
>>
>> Thanks, in advance, for any comments, questions or suggestions.
>>
>> Dan
>>
>>
>>
>>
>