wasm serialize & hook interface #4479

jerch · 2023-04-16T20:20:47Z

Early WIP/PoC...

jerch · 2023-04-16T21:25:01Z

@Tyriar The build passed in CI 😃 (seems my windows patch also works on azure containers...)

But the runtime penalty is quite big for mac and windows (>3 min), under linux it is only +20s 🤔

To not penalize things from the compiler sdk, I could change gitignore to include the built wasm file, thus the CI does not have to spin up a compiler at all. Downside - the wasm file would have to reside in the repo.

Edit: CI runtime is back to normal 😅 with the two commits below, but we now we might need an explicit force compile step before packing to not let slip through an old wasm binary by accident. Well CI setup still needs fine-tuning...

jerch · 2023-04-17T08:58:41Z

Running xterm-benchmark gives these numbers:

   Context "out-benchmark/benchmark/SerializeAddon.benchmark.js"
      Context "Terminal: sh -c "dd if=/dev/urandom count=40 bs=1k | hexdump | lolcat -f""
         Context "serialize"
            Case "#1" : 5 runs - average throughput: 5.91 MB/s

   Context "out-benchmark/benchmark/Serialize2Addon.benchmark.js"
      Context "Terminal: sh -c "dd if=/dev/urandom count=40 bs=1k | hexdump | lolcat -f""
         Context "serialize"
            Case "#1" : 5 runs - average throughput: 64.30 MB/s

jerch · 2023-04-18T21:07:20Z

@Tyriar Found a way to avoid costly SDK pulling during normal CI runs:

wasm target gets committed into repo by default
with inwasm -S recompilation can be force-skipped (compilation would pull SDKs), instead it just bundles & tests the repo version
later before packaging inwasm -f can be used to force a recompilation before doing final tests and packaging

Time to get back to the actual serialization code 😸

Tyriar · 2023-05-18T13:02:27Z

later before packaging inwasm -f can be used to force a recompilation before doing final tests and packaging

@jerch committing it and forcing compile during packaging sounds like a fine compromise to me. package and prepackage is always run before releasing. You could add the compile as postpackage as that should always run before we release, then just try remember to always commit it during PR changes so it stays up to date.

Tyriar · 2023-05-18T13:03:55Z

addons/xterm-addon-serialize2/README.md

@@ -0,0 +1,42 @@
+## xterm-addon-serialize
+
+An addon for [xterm.js](https://github.com/xtermjs/xterm.js) that enables xterm.js to serialize a terminal framebuffer into string or html. This addon requires xterm.js v4+.


v4 -> v5.2+? Should we just remove that line as there's the experimental line + we call it out in the release notes?

Sure thing. I just copy-cloned from the old addon and did not bother to fix all things yet.

Tyriar · 2023-05-18T13:07:30Z

addons/xterm-addon-serialize2/src-wasm/serialize.c

Can we add a "serialize2 (wasm)" button to the demo for convenient testing?

Yepp, already did that in my local branch (not pushed yet...)

Tyriar · 2023-05-18T13:09:18Z

addons/xterm-addon-serialize2/src/Serialize2Addon.test.ts

+  });
+
+  describe('text', () => {
+    // TODO: wirte alot more test cases here...


Is it possible to just copy over the old SerializeAddon.test.ts?

I did that, and removed all HTML test code (since the addon does not implement any HTML serialization yet). Then I realized, that there are no real test cases for the text-based serializer beside a dummy one... 😱

Tyriar · 2023-05-18T13:10:44Z

addons/xterm-addon-serialize2/src/Serialize2Addon.ts

+      throw new Error('Cannot use addon until it has been loaded');
+    }
+
+    return serialize((this._terminal as any)._core);


Haven't checked what you need here, but casting it to an internal interface like this will catch typing problems at compile time:

xterm.js/addons/xterm-addon-webgl/src/WebglAddon.ts

Line 38 in b7d73f6

const core = (terminal as any)._core as ITerminal;

Well that code is still the early hacking version (weirdly hacked in type conversions, no proper memory control, no nice interface yet). I hope you did read it too closely, or your eyes will bleed 😺

I am still messing with the right interface idea here - technically the wasm serialize part can act as a static singleton (would safe some resources/runtime later on during wasm bootstrapping), as JS does not allow multiple concurrent serializations from its single thread idea, and we also cannot asyncify things here, as we have to ensure, that the terminal buffer does not change, while a serialization is running.
So my current idea is to put everything behind a lazy static initializer, which will serve all serialization requests.

I didn't look too hard, just skimmed it 🙂

... did not read ..., tss tss tss 🤣

Tyriar · 2023-05-18T13:12:49Z

addons/xterm-addon-serialize2/src/serializer.wasm.ts

+// itoa LUT
+// note: performant decimal conversion of numbers is actually a hard problem
+// we use a LUT approach with 0-99 precomputed in utf16 for better performance
+const LUT100 = new Uint32Array(100);
+for (let i1 = 0; i1 < 10; ++i1) {
+  for (let i2 = 0; i2 < 10; ++i2) {
+    LUT100[i1 * 10 + i2] = (48 + i2) << 16 | (48 + i1);
+  }
+}


This isn't used other than d32.set(LUT100, 64)? We could either set it directly on d32 or put LUT100 in a closure such that we don't hold on to the Uint32Array

Yes, thats part of the bleeding eye code - see my remarks above.

Tyriar · 2023-05-18T13:13:28Z

addons/xterm-addon-serialize2/src/serializer.wasm.ts

+  // single call is pretty wasteful? preload combines once instead?
+  writeCombinedOverload = (dst, x) => {
+    let dp = dst >> 1;
+    const comb: string = (line as any)._combined[x] || '';


Any as any should use internal types where possible, if we need to expose a combined on the interface that's fine

Same as above 😺

Tyriar · 2023-05-18T13:14:27Z

addons/xterm-addon-serialize2/src/serializer.wasm.ts

+let writeCombinedOverload: (dst: number, x: number) => number = (dst, x) => dst;
+const writeCombined: (dst: number, x: number) => number = (dst, x) => writeCombinedOverload(dst, x);
+let writeLinkOverload: (dst: number, x: number) => number = (dst, x) => dst;


Instead of setting these functions via let, should they just be functions that take line and whatever else is needed?

Same as above 😺 - I already reworked that one locally, it is now a static function pointer on a context object and not on top level anymore.

Tyriar · 2023-05-18T13:14:45Z

addons/xterm-addon-serialize2/src/serializer.wasm.ts

+
+  const buffer = t.buffer;
+  const len = t.buffer.lines.length;
+  let wPos = 150*65536;


Surprised this isn't a lint rule:

Suggested change

let wPos = 150*65536;

let wPos = 150 * 65536;

Hmm, interesting. Yeah I fixed all linter nagging (was quite a bunch in my hacky code), but this one did not show up.

The hardcoded values will be replaced by proper memory management, prolly static again to avoid wasm compilation over and over, instead calling earlier into TextDecoder.decode.

jerch · 2023-05-23T21:11:12Z

reminder - also add overline attribute support.

jerch added 4 commits April 16, 2023 22:18

initial transfer

81168ea

run inwasm late

dce2d2c

fix test, disable linter for now

e4952d4

avoid .. in npm script

f205855

jerch added 3 commits April 16, 2023 23:31

add compiled wasm to repo to save CI runtime

ae1343d

add build files

27700c2

fix benchmark, add force to pack step

7a42219

jerch added 5 commits April 17, 2023 11:03

make linter happy

37ede60

upgrade inwasm

8e51c7e

preserve only needed built files

d20db7f

use external c file

8f466fc

use -S flag to avoid sdk pulling in CI steps

334f9f2

jerch added 2 commits April 19, 2023 17:12

upgrade inwasm, remove nonsense files in builddir

8c15eb2

some initial cleanup

840974d

Tyriar requested changes May 18, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wasm serialize & hook interface #4479

wasm serialize & hook interface #4479

jerch commented Apr 16, 2023 •

edited

jerch commented Apr 16, 2023 •

edited

jerch commented Apr 17, 2023

jerch commented Apr 18, 2023 •

edited

Tyriar commented May 18, 2023

Tyriar May 18, 2023

jerch May 18, 2023

Tyriar May 18, 2023

jerch May 18, 2023

Tyriar May 18, 2023

jerch May 18, 2023

Tyriar May 18, 2023

jerch May 18, 2023

Tyriar May 18, 2023

jerch May 18, 2023

Tyriar May 18, 2023

jerch May 18, 2023

Tyriar May 18, 2023

jerch May 18, 2023

Tyriar May 18, 2023

jerch May 18, 2023 •

edited

Tyriar May 18, 2023

jerch May 18, 2023 •

edited

jerch commented May 23, 2023

		@@ -0,0 +1,42 @@
		## xterm-addon-serialize

		An addon for [xterm.js](https://github.com/xtermjs/xterm.js) that enables xterm.js to serialize a terminal framebuffer into string or html. This addon requires xterm.js v4+.

wasm serialize & hook interface #4479

Are you sure you want to change the base?

wasm serialize & hook interface #4479

Conversation

jerch commented Apr 16, 2023 • edited

jerch commented Apr 16, 2023 • edited

jerch commented Apr 17, 2023

jerch commented Apr 18, 2023 • edited

Tyriar commented May 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerch May 18, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerch May 18, 2023 • edited

Choose a reason for hiding this comment

jerch commented May 23, 2023

jerch commented Apr 16, 2023 •

edited

jerch commented Apr 16, 2023 •

edited

jerch commented Apr 18, 2023 •

edited

jerch May 18, 2023 •

edited

jerch May 18, 2023 •

edited