Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support JS IR target #151

Merged
merged 4 commits into from
Jun 27, 2023
Merged

Conversation

fzhinkin
Copy link
Collaborator

Supported JS-IR target.

Closes #141

@fzhinkin fzhinkin requested review from qwwdfsad and shanshin June 27, 2023 10:25
@fzhinkin fzhinkin self-assigned this Jun 27, 2023
@fzhinkin fzhinkin linked an issue Jun 27, 2023 that may be closed by this pull request
@fzhinkin fzhinkin marked this pull request as ready for review June 27, 2023 10:26
Base automatically changed from prototype-preview-trimmed-down-api to prototype-preview June 27, 2023 10:39
@fzhinkin fzhinkin force-pushed the prototype-preview-js-src-set branch from ce62175 to 1806ed0 Compare June 27, 2023 11:53
Copy link
Contributor

@whyoleg whyoleg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor questions, mainly for future, not really necessary for now :)


import kotlinx.io.internal.commonAsUtf8ToByteArray

internal actual fun String.asUtf8ToByteArray(): ByteArray = commonAsUtf8ToByteArray()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: can't we just use String.encodeToByteArray(): ByteArray on all platforms?

Copy link
Collaborator Author

@fzhinkin fzhinkin Jun 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It (encodeToByteArray) behaves differently on JVM and Native when encoding invalid code points (and I'll revisit its usage in ByteString's).


package kotlinx.io

internal actual object SegmentPool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: will JS/Native have pooling some day? If so may be it's better to create an issue for this. Or this will be implemented in scope of #135/different segment types?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be considered after #135 (I expect to see the performance improvement on Native w/ enabled pooling, but I didn't measure it yet).

}
val bytesToRead = minOf(byteCount, (len - offset))
for (i in 0 until bytesToRead) {
sink.writeByte(buffer.readInt8(offset++) as Byte)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffer::copy expects Buffer or UInt8Array as the destination, and Segment's backing array (Segment::data) is an instance of Int8Array.

I think the issue could be addressed later, after #135

@fzhinkin fzhinkin merged commit 95d1e4c into prototype-preview Jun 27, 2023
@fzhinkin fzhinkin deleted the prototype-preview-js-src-set branch June 27, 2023 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support JS IR target
3 participants