RFR: 8305774: String.join(CharSequence, Iterable) can be optimized if Iterable is a Collection [v2]

[Chen Liang]

On Mon, 10 Apr 2023 05:19:05 GMT, Glavo <duke at openjdk.org> wrote:

>> @yuantj As I mentioned earlier, we need to prevent errors from leaking to other places, rather than defending against all errors.
>
>> @Glavo Then why doesn't `ArrayList` trust `toArray()`? If all implementations behave correctly, then it should return an `Object[]` independent of the original collection, so `ArrayList` should trust it. Those who doesn't implement `toArray()` correctly should use these methods at their own risk, shouldn't they?
> 
> I already explained the reason in detail in my earliest reply.
> 
>> This PR modifies a class that is used by nearly every JVM code, so safety is more important than performance.
> 
> Even without using `toArray`, security cannot be guaranteed. Just like the `BadList` in my previous example, if you cannot trust this collection, then we cannot do anything because its iterator may also be unsafe.

> @Glavo Then why doesn't `ArrayList` trust `toArray()`? If all implementations behave correctly, then it should return an `Object[]` independent of the original collection, so `ArrayList` should trust it. Those who doesn't implement `toArray()` correctly should use these methods at their own risk, shouldn't they?
> 
> This PR modifies a class that is used by nearly every JVM code, so safety is more important than performance.

@yuantj Because ArrayList stores that array. It doesn't want its state to mutate without calling collection mutator methods. Same goes for MethodType.methodTye. However, in our case, we are just using it momentarily and not storing the array from collection, so it's safe not to copy the array, like in MethodType.insertParameterTypes. If you really lacks trust, you can just convert to stream and call toArray, which always return a trusted array.

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

PR Comment: https://git.openjdk.org/jdk/pull/13383#issuecomment-1501408955