RFR: 8320359: ImageView: add styleable fitWidth, fitHeight, preserveRatio, smooth properties [v3]
Andy Goryachev
angorya at openjdk.org
Thu Nov 30 23:27:19 UTC 2023
On Thu, 30 Nov 2023 23:02:10 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
>> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
>>
>> save 8 bytes
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/CssUtil.java line 75:
>
>> 73:
>> 74: /** immutable list with random access backed by an array */
>> 75: private static class ImmutableArrayList<T> extends AbstractList<T> implements RandomAccess {
>
> I'd like to point that a skeletal implementation of `AbstractList` will perform worse than `ArrayList` for any method that needs to walk the list, as the `AbstractList` will use iterators for methods like `indexOf`, `hashCode` and `equals`. Now that will probably be irrelevant, but as this seems to be a micro optimization, you should be aware of all the trade offs.
Good point, thanks!
This also applies to UnmodifiableArrayList.
For completeness sake, I wanted to mention a few issues (not in scope for this PR) that came out of the code review:
- could use `UnmodifiableArrayList` but it stores extra int size. perhaps a factory method can be added to it for when `size != elements.length`.
- could improve UnmodifiableArrayList with fast(er) `indexOf`, `hashCode`, `equals` per @hjohn 's comment earlier
- `Control.getCssMetaData()` can be improved to skip merging of two lists if skinBase is null
- the same immutable list should be used in `Control.getCssMetaData()` instead of `ArrayList()`
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1293#discussion_r1411406802
More information about the openjfx-dev
mailing list