RFR (L/XXS) cleanup indent white space issues (8057107)
Coleen Phillimore
coleen.phillimore at oracle.com
Tue Sep 9 20:05:47 UTC 2014
Hi Dan,
I've looked through the changes in the patch file which is mind-numbing.
But I don't think you should make any of the changes to
sharedRuntime.cpp. I don't think the change like this is worth the
interference with others that have to merge up with this patch
(including myself). For example:
// Resolves a call. The compilers generate code for calls that go here
// and are patched with the real destination of the call.
methodHandle SharedRuntime::resolve_sub_helper(JavaThread *thread,
- bool is_virtual,
- bool is_optimized, TRAPS) {
+ bool is_virtual,
+ bool is_optimized, TRAPS) {
and
@@ -1213,8 +1213,8 @@
bool static_bound = call_info.resolved_method()->can_be_statically_bound();
KlassHandle h_klass(THREAD, invoke_code == Bytecodes::_invokehandle ? NULL : receiver->klass());
CompiledIC::compute_monomorphic_entry(callee_method, h_klass,
- is_optimized, static_bound, virtual_call_info,
- CHECK_(methodHandle()));
+ is_optimized, static_bound, virtual_call_info,
+ CHECK_(methodHandle()));
And:
switch (exception_kind) {
- case IMPLICIT_NULL: return Interpreter::throw_NullPointerException_entry();
- case IMPLICIT_DIVIDE_BY_ZERO: return Interpreter::throw_ArithmeticException_entry();
- case STACK_OVERFLOW: return Interpreter::throw_StackOverflowError_entry();
- default: ShouldNotReachHere();
+ case IMPLICIT_NULL: return Interpreter::throw_NullPointerException_entry();
+ case IMPLICIT_DIVIDE_BY_ZERO: return Interpreter::throw_ArithmeticException_entry();
+ case STACK_OVERFLOW: return Interpreter::throw_StackOverflowError_entry();
+ default: ShouldNotReachHere();
}
The case's can be indented from switch. I don't see anything wrong with
that and it's not worth the disruption to the code.
And I'm even going to have to merge with this! I think you should
revert sharedRuntime.cpp unless there are egregious formatting
violations that I didn't notice in the bulk of changes.
I thought thread.cpp would have the same merging difficulty for others,
but the changes seem to be to areas covered by contended locking
improvements (or small) and not likely to cause disruption.
Thanks,
Coleen
On 9/7/14, 3: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
>
>
More information about the hotspot-runtime-dev
mailing list