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

[Daniel Fuchs]

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
>