RFR: 8227054: ServiceThread needs to know about all OopStorage objects

[David Holmes]

Hi Coleen,

I've sat on this for a few hours :)

On 31/07/2019 8:15 am, coleen.phillimore at oracle.com wrote:
> On 7/30/19 5:11 PM, David Holmes wrote:
>> On 31/07/2019 6:59 am, Kim Barrett wrote:
>>>> On Jul 29, 2019, at 10:27 PM, David Holmes <david.holmes at oracle.com> 
>>>> wrote:
>>>>
>>>> Hi Kim,
>>>>
>>>> A meta-comment: "storages" is not a well formed term. Can we have 
>>>> something clearer, perhaps OopStorageManager, or something like that?
>>>>
>>>> Thanks,
>>>> David
>>>
>>> Coleen suggested the name OopStorages, as the plural of OopStorage.
>>
>> "storage" doesn't really have a plural in common use.
> 
> Well this isn't common use.  There are more than one oopStorage things 
> in oopStorages.
>>
>>> (Unpublished versions of the change had a different name that I didn't
>>> really like and Coleen actively disliked.)  Coleen and I both have an
>>> antipathy toward "Manager" suffixed names, and I don't see how it's
>>> any clearer in this case.  "Set" suggests a wider API.
>>>
>>> Also, drive-by name bikeshedding doesn't carry much weight.
>>
>> Okay how about its really poor form to have classes and files that 
>> differ by only one letter. I looked at this to see what it was about 
>> and had to keep double-checking if I was looking at OopStorage or 
>> OopStorages. In addition OopStorages conveys no semantic meaning to me.
>>
> 
> This might be confusing to someone who doesn't normally look at the 
> code.  

The fact they differ by only one letter leads to an easy source of 
mistakes in both reading and writing the code. The very first change I 
saw in the webrev was:

   #include "gc/shared/oopStorage.inline.hpp"
+ #include "gc/shared/oopStorages.hpp"

and I immediately thought it was a mistake because the .hpp would be 
included by the .inline.hpp file - but I'd missed the 's'.

> If you come up with a better name than Manager, it might be okay 
> to change.  So far, our other name ideas weren't better than just the 
> succinct "Storages".   Meaning multiple oopStorage objects (they're not 
> objects, that's a bad name because it could be confusing with oops which 
> are also called objects).

OopStorageUnit
OopStorageDepot
OopStorageFactory
OopStorageHolder
OopStorageSet

Arguably this could/should be folded into OopStorage itself and avoid 
the naming issues altogether.

Cheers,
David

P.S. What's so bad about Manager? :)

> Coleen
> 
>> Thanks,
>> David
>