RFR(xs): 8059361: Properties.stringPropertyNames() returns a set inconsistent with the assertions from the spec

[Mandy Chung]

On May 26, 2016, at 3:49 PM, Stuart Marks <stuart.marks at oracle.com> wrote:
> 
> 
> 
> On 5/26/16 2:28 PM, Mandy Chung wrote:
>>> -        return h.keySet();
>>> +        return Set.of(h.keySet().toArray(new String[0]));
>>>    }
>> 
>> The patch looks fine.  It’d be good to add a test case.
>> 
>> If you use Collections.unmodifiableSet, you would not need to convert the key sets to an array. Any benefit of using Set::of instead of Collections.unmodifiableSet?
> 
> There are several minor tradeoffs.
> 
> Collections.unmodifiableSet() simply creates and returns a wrapper object. It contains the keySet, which is backed by the HashMap created within the stringPropertyNames() method. This keeps references to all the string keys and values around, even though the values aren't used. But the strings themselves are also referenced from the original Properties object and its default chain.
> 
> The toArray() call copies the key string refs to an array, then Set.of() hashes them into its internal array. But the resulting data structure holds only references to the keys, so it's more compact.
> 
> Given that most uses seem to iterate the set's elements and then throw it away, it's probably not worth the creation overhead of for an immutable Set. Drat. I guess I was too eager to use the new API. :-)
> 
> I've changed this to use Collections.unmodifiableSet(); see below.
> 

Looks fine.  

I don’t have any objection in using Set::of vs Collections.unmodifiableSet() in this case (Sorry.  I should have made it clear).  No need for new patch if you decide to use Set.of (a comment might help).

Mandy