On Thu, 31 Oct 2024 20:36:28 GMT, Harshitha Onkar <hon...@openjdk.org> wrote:
>> If a thread-safe implementation is not needed, it is recommended to use
>> HashMap instead of legacy synchronized Hashtable.
>> Map `CSS.htmlAttrToCssAttrMap` is modified only within static initialization
>> block and then only `get` method is called.
>>
>> The only subtle difference is that Hashtable doesn't allow `null` keys and
>> throws NPE from `get` method.
>> I've checked all possible keys which are passed to
>> `htmlAttrToCssAttrMap.get`.
>> Currently 3 different execution paths are possible:
>>
>> **1,2**
>> When `HTML.Attribute.BORDER` is passed as a key. It's definitely non-null.
>>
>> javax.swing.text.html.CSS#translateHTMLToCSS
>> translateAttribute(HTML.Attribute.BORDER, "1", cssAttrSet);
>> CSS.Attribute[] cssAttrList = getCssAttribute(key);
>>
>>
>> javax.swing.text.html.CSS#translateAttributes
>> translateAttribute(HTML.Attribute.BORDER, Integer.toString(borderWidth),
>> cssAttrSet);
>> CSS.Attribute[] cssAttrList = getCssAttribute(key);`
>>
>>
>> **3**
>> When non-null `key` is passed as a key.
>>
>> javax.swing.text.html.CSS#translateAttributes
>>
>> if (name instanceof HTML.Attribute) { // from this `instanceof` we know
>> that it's non-null
>> HTML.Attribute key = (HTML.Attribute)name;
>>
>> translateAttribute(key, (String) htmlAttrSet.getAttribute(key),
>> cssAttrSet);
>> CSS.Attribute[] cssAttrList = getCssAttribute(key);
>>
>>
>> 
>>
>>
>> I've used HashMap.newHashMap method instead of constructor to avoid resizing
>> of internal HashMap array.
>
> @turbanoff Based on your explanation, it sounds reasonable to convert
> CSS.htmlAttrToCssAttrMap from Hashtable to HashMap since synchronization may
> not be required.
>
> There are few more Hashtables - `styleConstantToCssMap,
> htmlValueToCssValueMap, cssValueToInternalValueMap`. Do we consider
> converting them as well - either to HashMap or ConcurrentHashMap (if
> thread-safe synchronization is required)?
@honkar-jdk yes. It was my intention to convert them all. I want to go
one-by-one to make review easier.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/21785#issuecomment-2450779444
