RFR[8246047]: 'Replace LinkedList impl in net.http.websocket.BuilderImpl'

[Conor Cleary]

Thanks for the suggestion Daniel, looks much nicer without the casting 
in the immutableCopy function as well!

I made a new patch with those reverted changes and it looks much nicer 
and still behaves as expected.

webrev: 
http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8246047/webrevs/webrev.03/


Thanks!

Conor


On 14/08/2020 15:49, Daniel Fuchs wrote:
> Hi Conor,
>
> Thanks for addressing this technical debt.
>
> I don't think this is correct. You transformed the immutable
> copy at line 137 into a shallow clone, since now both copies
> share the same mutable list.
>
> I suggest to revert the changes at lines:
>
> 66,67,144 and 145.
>
> (and that will be much simpler in the bargain :-) )
>
> best regards,
>
> -- daniel
>
>
> On 14/08/2020 15:37, Conor Cleary wrote:
>> Hi all,
>>
>> Requesting some reviewers for a patch concerning JDK-8246047 'Replace 
>> LinkedList impl in net.http.websocket.BuilderImpl'. This patch 
>> replaces LinkedList data structures used in the net.http.websocket 
>> BuilderImpl class with ArrayLists. In particular, the 'headers' and 
>> 'subprotocols' Collections in the class are now assigned ArrayLists 
>> in the BuilderImpl constructors.
>>
>> Some justifications for this change are as follows:
>>
>>   * Sequential Access Times for ArrayLists are improved due to locality
>>     of reference (i.e ArrayList elements stored in same neighbourhood)
>>   * Get(index) operations are O(1) time complexity for ArrayLists as
>>     opposed to worst-case O(N-1) for LinkedLists
>>   * While insertion operations can be expensive (O(N) in the worst
>>     case), the 'headers' and 'subprotocols' lists are not modified after
>>     creation and so this is of minor concern
>>
>> Additional justifications or challenges to them are welcome! The 
>> general idea is that ArrayLists out-perform LinkedLists in this 
>> particular scenario.
>>
>>   * webrev:
>> http://cr.openjdk.java.net/~ccleary/issues/webrevs-store/8246047/webrevs/webrev.02/
>>   * bug: https://bugs.openjdk.java.net/browse/JDK-8246047
>>
>>
>> Conor
>>
>