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

Embedded Swift support [WIP] #251

Closed
wants to merge 2 commits into from
Closed

Conversation

MihaelIsaev
Copy link

@MihaelIsaev MihaelIsaev commented Jun 10, 2024

It adds an ability to use the original JavaScriptKit in Embedded Swift

All the targets has been copied with Embedded suffix and uses the original source code via symlinks in order to have an ability to add special flags for Embedded Swift.

String16 library has been written with an absolute minimal working code at this moment in order to provide UTF-16 String replacement for Embedded Swift.

EmbeddedFoundation library provides malloc, free, arc4rand methods for Embedded Swift.

What's not working currently: JSClosure, JSOneShotClosure. It seems I missing something. Investigating what's wrong.

LightWebApp is a demo Embedded Swift project which uses this version of JavaScriptKit.

@tierracero
Copy link

tierracero commented Jun 10, 2024

Wow awsome update. Hope to see it soon.

Copy link
Member

@kateinoigakukun kateinoigakukun left a comment

Choose a reason for hiding this comment

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

First of all, thank you for bringing this up! General feedbacks:

  1. Why do we need String16 instead of StaticString? If I remember correctly, we don't need much string manipulation but just passing around it. So StaticString should be enough for us.
  2. I'd like to keep this package standalone without any external dependency to have maintenance flexibility. Also it's a risk for us to add dependencies without explicit license.
  3. Could you reduce number of code duplication? Also I think it would be great to incrementally unlock a subset of API for Embedded target instead of trying to unlock whole APIs. It would allow me to review easily and move this forward quickly.

@MihaelIsaev
Copy link
Author

@kateinoigakukun thank you very much for the quick review!

Number of code duplication is mostly because of moving from ConvertibleToJSValue to JSValue. Since any is prohibited in Embedded Swift, normal usage of ConvertibleToJSValue is impossible, only explicit JSValue works.

Without String16 it is possible to move from String to just StaticString everywhere, so it will reduce amount of code duplication.
Alternatively, we could use a global typealias as follows:

#if hasFeature(Embedded)
public typealias Str = String16
#else
public typealias Str = String
#endif

When I started working with Embedded Swift + WebAssembly a month ago I asked a question regarding excluded String support, and @MaxDesiatov mentioned re-encoding to and from UTF-16 overhead. Maybe I understood his comment wrong, but I decided to try to write UTF-16 driven String.

Keeping this package standalone is important, I agree. Though something like dlmalloc is still required with Embedded Swift, should it be built-in? I guess if dlmalloc will be built-in then if somebody would want to use it in other package he will either use it via extern C declarations or will face with duplicate symbols issue. That's why I moved it into the separate package.

String16 and EmbeddedFoundation do not have licenses yet because they are still drafts, but I usually use the MIT license. If String16 and EmbeddedFoundation have the potential to be included in JavaScriptKit, I would be eager to transfer them to the swiftwasm organization and with the team’s permission I would also like to maintain them.
I am passionate about Swift for WebAssembly, and my dream is to compile swift into wasm files that are as compact as possible, yet fully functional.

Also I think it would be great to incrementally unlock a subset of API for Embedded target instead of trying to unlock whole APIs.

Do you mean that it'd be better to have completely separate Embedded target with its own code?

@kateinoigakukun
Copy link
Member

kateinoigakukun commented Jun 11, 2024

Number of code duplication is mostly because of moving from ConvertibleToJSValue to JSValue. Since any is prohibited in Embedded Swift, normal usage of ConvertibleToJSValue is impossible, only explicit JSValue works.

Ah, I see. That's an unfortunate situation... Let me think more on how the current API could fit on Embedded target.

When I started working with Embedded Swift + WebAssembly a month ago I asked a question regarding excluded String support, and @MaxDesiatov mentioned re-encoding to and from UTF-16 overhead. Maybe I understood his comment wrong, but I decided to try to write UTF-16 driven String.

Even though we use UTF-16 as a representation in Swift memory space, taking a string from Swift to JS still require copying buffer and re-interpret byte sequence.

And also transcoding UTF-8 into UTF-16 is not difficult to optimize with fast path where it fits within ASCII range. I took a quick benchmark on Node.js and it looks like decoding UTF-8 within ASCII range is noticeably faster than UTF-16 case.

Even though it's slower for long non-ASCII chars, but most Web APIs are named within ASCII range, so it still makes sense to keep UTF-8 even on Embedded target.

### Decode Performance ###
ASCII chars (fast-path) utf-8   171.40ms
ASCII chars (fast-path) utf-16  504.52ms
Non-ASCII chars         utf-8   13320.24ms
Non-ASCII chars         utf-16  1975.70ms

### Encode Performance ###
ASCII chars (fast-path) utf-8   105.63ms
ASCII chars (fast-path) utf-16  97.88ms
Non-ASCII chars         utf-8   4751.53ms
Non-ASCII chars         utf-16  4699.72ms
function decodePerformance(data, encoder, decoder) {
  // Encode the data
  const encoded = encoder.encode(data);

  const start = performance.now();
  // Decode the data many times
  for (let i = 0; i < 1000; i++) {
    decoder.decode(encoded);
  }
  const end = performance.now();
  return end - start;
}

function encodePerformance(data, encoder) {
  const start = performance.now();
  // Encode the data many times
  for (let i = 0; i < 1000; i++) {
    encoder.encode(data);
  }
  const end = performance.now();
  return end - start;
}

for (const [direction, doit] of [
  ['Decode', (data, encoding) => decodePerformance(data, new TextEncoder(encoding), new TextDecoder(encoding))],
  ['Encode', (data, encoding) => encodePerformance(data, new TextEncoder(encoding))],
]) {
  console.log(`\n### ${direction} Performance ###`);
  for (const [charType, data] of [
    ['ASCII chars (fast-path)', 'a'.repeat(1000000)],
    ['Non-ASCII chars        ', '🦄'.repeat(1000000)],
  ]) {
    for (const encoding of ['utf-8', 'utf-16']) {
      const time = doit(data, encoding);
      const formattedTime = time.toFixed(2);
      const formattedEncoding = encoding.padEnd(7);
      console.log(`${charType} ${formattedEncoding} ${formattedTime}ms`);
    }
  }
}

Keeping this package standalone is important, I agree. Though something like dlmalloc is still required with Embedded Swift, should it be built-in? I

We need a malloc implementation for testing but JavaScriptKit package should not ship it. We should leave the allocator choice to the user decision.

Do you mean that it'd be better to have completely separate Embedded target with its own code?

Sorry, I meant it would be a good first step if we guard out most of source files from the new ~Embedded targets by exclude: [...] and #if hasFeature(Embedded), then unlock a small part of core APIs like JSObject incrementally. TBH, the current patch set is so large that I can't review carefully

Brings back `JSClosure` and `JSFunction` to life
@MihaelIsaev
Copy link
Author

74a45ec JSClosure and JSFunction now works properly

@kateinoigakukun
Copy link
Member

Closing in favor of #263

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.

3 participants