Implement JSString to reduce bridging overhead #63

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

Jump to bottom

Merged

kateinoigakukun merged 2 commits into master from js-string

Sep 18, 2020

Merged

Implement JSString to reduce bridging overhead #63

kateinoigakukun merged 2 commits into master from js-string

Sep 18, 2020

Conversation

@kateinoigakukun

Copy link

Member

@kateinoigakukun kateinoigakukun commented Sep 18, 2020 •

edited

Loading

Before

node bin/benchmark-tests.js
Running 'Serialization/Write JavaScript number directly' ...
done 123.83756804466248 ms
Running 'Serialization/Write JavaScript string directly' ...
done 129.04579210281372 ms
Running 'Serialization/Swift Int to JavaScript' ...
done 3677.7331490516663 ms
Running 'Serialization/Swift String to JavaScript' ...
done 5490.698668003082 ms
Running 'Object heap/Increment and decrement RC' ...
done 3000.8530011177063 ms

After

node bin/benchmark-tests.js
Running 'Serialization/Write JavaScript number directly' ...
done 130.88212418556213 ms
Running 'Serialization/Write JavaScript string directly' ...
done 137.1980230808258 ms
Running 'Serialization/Swift Int to JavaScript' ...
done 3526.049220085144 ms
Running 'Serialization/Swift String to JavaScript' ...
done 3598.435418844223 ms
Running 'Object heap/Increment and decrement RC' ...
done 2782.3261749744415 ms

@kateinoigakukun


 Implement JSString to reduce briding overhead

77985a6

@kateinoigakukun kateinoigakukun mentioned this pull request

Sep 18, 2020

Remove payload2 from value bridging interface #64

Merged

@kateinoigakukun


 Bump runtime library version

ce012f4

MaxDesiatov

MaxDesiatov reviewed

Sep 18, 2020

View reviewed changes

Sources/JavaScriptKit/FundamentalObjects/JSString.swift

/// - lhs: A string to compare.

/// - rhs: Another string to compare.

public static func == (lhs: JSString, rhs: JSString) -> Bool {

return lhs.guts.buffer == rhs.guts.buffer

Copy link

Member

@MaxDesiatov MaxDesiatov Sep 18, 2020

Choose a reason for hiding this comment

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

Could there be a way to run this comparison on the JS side, without potentially triggering copies?

Copy link

Member Author

@kateinoigakukun kateinoigakukun Sep 18, 2020

Choose a reason for hiding this comment

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

That's a trade-off point. If we compare them in JS side, it may trigger copies and re-encode Swift string to JS

Copy link

Member

@j-f1 j-f1 Sep 18, 2020

Choose a reason for hiding this comment

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

Does it make sense to add a flag to Guts that reports whether the value is cached in Swift? That way it would be possible to do the check in JS if neither string is cached in Swift.

MaxDesiatov

MaxDesiatov approved these changes

Sep 18, 2020

View reviewed changes

Copy link

Member

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Great stuff! 👏

@kateinoigakukun kateinoigakukun merged commit 36a06c5 into master

Sep 18, 2020

@kateinoigakukun kateinoigakukun deleted the js-string branch

September 18, 2020 11:06

j-f1

j-f1 reviewed

Sep 18, 2020

View reviewed changes

Sources/JavaScriptKit/FundamentalObjects/JSString.swift

}()

lazy var buffer: String = {

var bytesRef: JavaScriptObjectRef = 0

Copy link

Member

@j-f1 j-f1 Sep 18, 2020

Choose a reason for hiding this comment

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

Suggested change

var bytesRef:JavaScriptObjectRef=0

var bytesRef=JavaScriptObjectRef()

Sources/JavaScriptKit/FundamentalObjects/JSString.swift

/// - lhs: A string to compare.

/// - rhs: Another string to compare.

public static func == (lhs: JSString, rhs: JSString) -> Bool {

return lhs.guts.buffer == rhs.guts.buffer

Copy link

Member

@j-f1 j-f1 Sep 18, 2020

Choose a reason for hiding this comment

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

Does it make sense to add a flag to Guts that reports whether the value is cached in Swift? That way it would be possible to do the check in JS if neither string is cached in Swift.

Labels

None yet

4 participants

@kateinoigakukun @MaxDesiatov @j-f1

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Implement JSString to reduce bridging overhead #63

Implement JSString to reduce bridging overhead #63

Uh oh!

Conversation

@kateinoigakukun kateinoigakukun commented Sep 18, 2020 •

edited

Loading

Uh oh!

Uh oh!

@MaxDesiatov MaxDesiatov Sep 18, 2020

Choose a reason for hiding this comment

Uh oh!

@kateinoigakukun kateinoigakukun Sep 18, 2020

Choose a reason for hiding this comment

Uh oh!

@j-f1 j-f1 Sep 18, 2020

Choose a reason for hiding this comment

Uh oh!

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

Uh oh!

@j-f1 j-f1 Sep 18, 2020

Choose a reason for hiding this comment

Uh oh!

@j-f1 j-f1 Sep 18, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Implement JSString to reduce bridging overhead #63

Implement JSString to reduce bridging overhead #63

Uh oh!

Conversation

@kateinoigakukun kateinoigakukun commented Sep 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

@MaxDesiatov MaxDesiatov Sep 18, 2020

Choose a reason for hiding this comment

Uh oh!

@kateinoigakukun kateinoigakukun Sep 18, 2020

Choose a reason for hiding this comment

Uh oh!

@j-f1 j-f1 Sep 18, 2020

Choose a reason for hiding this comment

Uh oh!

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

Uh oh!

@j-f1 j-f1 Sep 18, 2020

Choose a reason for hiding this comment

Uh oh!

@j-f1 j-f1 Sep 18, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

@kateinoigakukun kateinoigakukun commented Sep 18, 2020 •

edited

Loading