Skip to content
This repository has been archived by the owner on Jul 8, 2022. It is now read-only.

Have *Array2 classes implement an interface [Includes small breaking change] #616

Merged
merged 5 commits into from
May 5, 2022

Conversation

Kietyo
Copy link
Contributor

@Kietyo Kietyo commented May 2, 2022

This will allow me to add more useful extension functions in the future without have to redundantly implement it for each type.

There is a small breaking change.

I changed toString(...) method names to asString since I found refactoring it broke the JS IR compiler. I don't think this will be that big of change because the no-parameter toString() implementation is still the same as before, and that's probably what will be used the most.

Clients can simply rename toString -> asString if they're using the overloaded versions.

Xeno added 2 commits May 1, 2022 21:41
…ier to extend with type agnostic array2 extension functions.
@soywiz
Copy link
Member

soywiz commented May 2, 2022

Nice. But we should be aware of the performance regression due to boxing.

Also what's the JS bug? If we circumvent it, we should also probably report it. What's the stacktrace?

@Kietyo
Copy link
Contributor Author

Kietyo commented May 3, 2022

Thanks a lot for the comments, I'm learning a lot! I also saw your post a few years back:

https://discuss.kotlinlang.org/t/performance-question-related-to-boxing-and-interface-implementation/17387/3

I decided to go with revert the get/set changes and introduce the getAt and setAt methods that you mention.

Next I'm working on a potential workaround to avoid extra allocs for the general methods (e.g each, fill), which is to introduce an auxiliary data class that can be used to passing data around without boxing:

data class Array2Element(
    var intData: Int = 0,
    var floatData: Float = 0f,
    var doubleData: Double = 0.0,
    var any: Any? = null
)

I took a look at the generated Java code, and noticed it doesn't box:

image

Notice that getAt boxes, but getTo doesn't box.

I'll work more on it tomorrow.

@Kietyo
Copy link
Contributor Author

Kietyo commented May 4, 2022

Ok NVM the idea with using an auxiliary data didn't work. Ready for review.

@soywiz soywiz merged commit 6987088 into soywiz-archive:main May 5, 2022
@soywiz
Copy link
Member

soywiz commented May 5, 2022

IArray2<Thanks>!

@Kietyo Kietyo deleted the new_stuff2 branch May 13, 2022 21:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants