RFR (S): 8214791: Rename vm_operations_g1* files to g1VMOperations*

[David Holmes]

Hi Thomas,

I like the turn this has taken :)

On 6/12/2018 1:05 am, Thomas Schatzl wrote:
> Hi,
> 
> On Wed, 2018-12-05 at 12:41 +0100, Per Liden wrote:
>> Hi,
>>
> [...]
>>>> Coleen and I talked about this, and neither of us wants yet
>>>> another naming convention for these files.
>>>> See her comment in the bug report for her
>>>> preference.  (vmG1Operations).  I don’t care that much
>>>> about the specifics, so long as there is consistency (which there
>>>> admittedly is not, at present, but
>>>> this proposed change just makes that worse).
>>>>
>>>
>>> I could rename the other gc files too to make them consistent, e.g.
>>> cmsVMOperations :)
>>>
>>> However I can be convinced to go with vmG1Operations although it
>>> does read backwards imho. At least it gets rid of the underscore.
>>>
>>> New webrev with that latter change:
>>> http://cr.openjdk.java.net/~tschatzl/8214791/webrev.1
>>
>> I don't know... I would suggest we have either a prefix or a suffix.
>> The "midfix" (is that a word?) style is just strange. 

"infix" is the word. Yes infix is just plain weird here. :)

>> For example:
>>
>>    runtime/vmOperations.hpp
>>    gc/shared/gcVMOperations.hpp
>>    gc/g1/g1VMOperations.hpp
>>    gc/parallel/psVMOperations.hpp
>>    gc/cms/cmsVMOperations.hpp

Much better.

I'm assuming there are reasons we have to repeat the "name" part in the 
file name even though it's uniquely determined by the directory path? 
Afterall:

#include <g1/g1Foo.hpp>

does look somewhat redundant versus

#include <g1/foo.hpp>

> I would prefer this (this is actually the original suggestion) and will
> do the work if everyone agrees on that. Widening to hotspot-runtime-dev
> in the hope that we can catch all that discussion there too then.
> 
>> or maybe (to follow the vmStructs style):
>>
>>    runtime/vmOperations.hpp
>>    gc/shared/vmOperations_gc.hpp
>>    gc/g1/vmOperations_g1.hpp
>>    gc/parallel/vmOperations_ps.hpp
>>    gc/cms/vmOperations_cms.hpp

This works for me too - I have no prejudices against underscores :)

>>
>> But then I also don't think there is any real value in always having
>> VM_Operation (or other things like OopClosure) classes declared in
>> separate files, like we currently do. In some cases such classes
>> have very little in common and are better place in the context they
>> are used.
>> For example, in ZGC the VM_ZOperation class is an internal part of
>> the ZDriver, and not declared in a separate file.
> 
> I agree, I saw that and think it is nice(r) :) However e.g. the
> g1CollectedHeap.cpp file is big enough as it is, and the files are
> there already.

Probably a case by case concern. I certainly don't see a need to always 
factor out the vm-ops, but if you want all the different GC's to have a 
consistent file naming/structure then its fine.

> It's just that every time I look at these names in the file browser I
> see these very strangely named files and think to myself that this
> should be fixed. Now here we are...

I feel grateful that I rarely have to delve into a gc subdirectory ;-)

Thanks,
David

> Thanks,
>    Thomas
> 
>