RFR: 8343418: Unnecessary Hashtable usage in CSS.htmlAttrToCssAttrMap [v4]

Alexey Ivanov aivanov at openjdk.org
Mon Dec 2 16:51:40 UTC 2024


On Tue, 12 Nov 2024 20:28:55 GMT, Andrey Turbanov <aturbanov at 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);
>> 
>> 
>> ![idea_analyze_dataflow_to_here](https://github.com/user-attachments/assets/48ace4de-6d0a-468e-bb14-b579a193254b)
>> 
>> 
>> I've used HashMap.newHashMap method instead of constructor to avoid resizing of internal HashMap array.
>
> Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8343418: align more

Looks good to me.

I ran client tests once again, the tests pass.

-------------

Marked as reviewed by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21785#pullrequestreview-2473298051


More information about the client-libs-dev mailing list