makes FS.read async, so you can read asynchronously from stdin #3714

albertjan · 2015-08-25T18:36:35Z

This will make things like #23 (an issue from the way back when machine) a lot easier. It will also make the stdin a better browser citizen.

This implementation supports both sync and async input from the stdin. As you can see in the test. The buffer is filled with a promise that also returns the first byte and the rest of the buffer is returned to the read function immediately.

I have some failing tests but as far as I can see most of them are because I didn't have java installed.

testoutput

kripken · 2015-08-27T23:14:44Z

What is on the stack when read() is called? (if it's anything nontrivial, that might break, as we need to reconstruct the call stack, and can only do it for compiled code)

I see this depends on Promise - I'm worried about browser support, at least IE11 I believe doesn't have it.

albertjan · 2015-08-28T07:18:34Z

What is on the stack when read is called depends on what you compile right? We've been using it in pypy which is pretty big. I don't know enough about C and emscripten to answer this question.

I guess you're right making it use EmterpreterAsync will result in it only being available when called from code compiled with Emterpreter. If you would want to use in the javascript world you'd have to compile a wrapper around it.

You're right Promise support is not available in IE11, although easily polyfillable. But I could change it to use callbacks, That will make it a few lines longer though.

Is this something you would consider? The pull request I mean 😄

kripken · 2015-09-03T21:00:43Z

My concern is the code in between the C code and the read() call. Can you print out the call stack (stackTrace()) and paste that here?

If there is any JS code in the middle, we need to be sure it is safe. In general, we only call EmterpreterAsync.handle from a direct JS library call, i.e., the target of a C call to a library. But here there is stuff in the middle.

It might be safer to put the async stuff at the syscall level, look in src/library_syscall.js for other EmterpreterAsync.handle, we have one on fsync. Perhaps one on read would work here.

I would definitely consider this pull, assuming the safety issue is resolved. I think we should avoid Promise, much as it would be nice to support it.

albertjan · 2015-09-04T20:21:28Z

This is the stack:

so only _read is between it.

Error
    at jsStackTrace (blob:http%3A//52.6.7.231%3A3030/57073b5a-2c96-4206-b55b-ade228782e79:1161:13)
    at stackTrace (blob:http%3A//52.6.7.231%3A3030/57073b5a-2c96-4206-b55b-ade228782e79:1178:22)
    at Object.eval (eval at evaluate (unknown source), <anonymous>:1:1)
    at Object.InjectedScript._evaluateOn (<anonymous>:904:55)
    at Object.InjectedScript._evaluateAndWrap (<anonymous>:837:34)
    at Object.InjectedScript.evaluateOnCallFrame (<anonymous>:963:21)
    at Object.FS.createDevice.FS.registerDevice.read (blob:http%3A//52.6.7.231%3A3030/57073b5a-2c96-4206-b55b-ade228782e79:4847:6)
    at Object.FS.read (blob:http%3A//52.6.7.231%3A3030/57073b5a-2c96-4206-b55b-ade228782e79:4492:43)
    at _read (blob:http%3A//52.6.7.231%3A3030/57073b5a-2c96-4206-b55b-ade228782e79:6430:19)
    at emterpret (blob:http%3A//52.6.7.231%3A3030/57073b5a-2c96-4206-b55b-ade228782e79:214113:11)"

albertjan · 2015-09-04T20:25:36Z

I will look into the syscall

kripken · 2015-09-04T20:55:52Z

Ok. If it's just read, maybe it's not too bad. But any future refactoring might make it worse. Doing it at the syscall level should be future-proof.

albertjan · 2015-09-07T19:46:33Z

So I figured out what doing it in syscall means 😄 I think. I will update accordingly. I'm going to make the stream in __syscall3 capable of handling callbacks and promises. that would make FS.read "synchronous" again.

albertjan · 2015-09-07T20:01:48Z

I should think before I type. 😄 I will also have to change the way FS.read works because thats where I have access to the input method. I should make sure the EmterptretifyAsync stuff happens in __syscall3.

kripken · 2015-09-10T20:18:09Z

src/library_syscall.js

+    var stream = SYSCALLS.getStreamFromFD(), buf = SYSCALLS.get(), count = SYSCALLS.get();
+    return EmterpreterAsync.handle(function (resume) {
+      var result = FS.read(stream, {{{ makeGetSlabs('buf', 'i8', true) }}}, buf, count, null, function (bytesRead) {
+        resume(function() { bytesRead });


needs return

albertjan · 2015-09-10T21:37:05Z

Ah yeah hi I've been at it a bit more. And I've found I also need to implement syscall 145 and there is also pread in there somewhere 😄. There's also something else broken still because allthough the test pases when I run it seperatelly it breaks. I'm away for the weekend and will be back at it Monday. I should probably not have pushed this yet. I'll let you know when I thinks it's done.

albertjan · 2015-09-17T20:17:08Z

@kripken I've rebased it and added the EmterpreterAsync calls to library_syscall. I've added a test for readv it uses gets which is considered bad practice I think but I don't know if it really matters for the tests.

kripken · 2015-09-21T20:16:02Z

src/library_syscall.js

@@ -808,10 +861,21 @@ var SyscallsLibrary = {
    }
    return nonzero;
  },
+#if EMTERPRETIFY_ASYNC
+  __syscall180: function(which, varargs) { // pread64
+    var stream = SYSCALLS.getStreamFromFD(), buf = SYSCALLS.get(), count = SYSCALLS.get();


what happened to the zero at the end, seen in the non-async version below?

kripken · 2015-09-21T20:17:42Z

Overall this looks functional, but the code duplication is a concern, as mentioned above. But also, in general this is more invasive than I had expected. Perhaps we need to do some refactoring to make things like this easier to do? Feels like we are doing something wrong here.

albertjan · 2015-09-21T20:49:08Z

I've added the count and offset back in. I've also removed the obvious duplications. I'll have to look into making it easier, but because we want to enter the async world in library_syscall.js I needed to add callbacks at a lot of places. The shortest path to makt this easier would be to make use of Promises 😄

kripken · 2015-09-23T18:54:55Z

src/library_syscall.js

@@ -810,7 +857,15 @@ var SyscallsLibrary = {
  },
  __syscall180: function(which, varargs) { // pread64
    var stream = SYSCALLS.getStreamFromFD(), buf = SYSCALLS.get(), count = SYSCALLS.get(), zero = SYSCALLS.getZero(), offset = SYSCALLS.get64();
+#if EMTERPRETIFY_ASYNC


perhaps one way to reduce code duplication in places like this is if we always passed the extra parameter to FS.read, but it was just ignored when not async. And, if we automatically added the code to add the handle, for a list of async-aware syscalls, by modifying the text. We already do similar things for SYSCALL_DEBUG at the bottom of this file.

Sure I'll try it out see how that would look/work.

kripken · 2015-10-07T23:53:28Z

Another thought on this: Perhaps we could do sync stdin reading through a new API, say emscripten_sync_stdin()? Not doing it via stdio's intefaces and the filesystem code, just directly reading from Module.stdin, might be simple but still useful.

albertjan · 2015-10-08T07:16:45Z

Ok that would make things a lot less intricate/duplicated. 😄 you'd want to make the async version the default?

It might be a good idea to add a few more api's to the sync one that might make it easier for people to port their existing sync code to the new api. I however don't know enough about c/c++ to make any assumptions about which those would be 😄

With permission from my employer/client I will revisit this PR soon, but it won't be this or next week.

kripken · 2015-10-08T18:26:53Z

I was thinking that there would be just a async version for this, unless there is value in a regular one too? I'm fine with other new APIs that are useful, of course.

albertjan · 2015-10-08T19:09:59Z

Yeah sounds good to me. :)

albertjan · 2015-10-11T12:26:16Z

I think I've just run into a little problem 😄 this also makes all the other reads async. And for my case pypyjs forces me to add a lot of functions to the EMTERPRETIFY whitelist. I think we may need to rethink how to go about this.

kripken · 2018-12-07T01:21:23Z

This PR is quite old, closing. Feel free to reopen if still relevant.

zone117x · 2019-07-20T10:10:19Z

@kripken Any suggestions for how to accomplish this today? (Feeding data into stdin via async js)
I'm using the Rust emscripten toolchain.

kripken · 2019-07-21T23:16:49Z

@zone117x In general it should be easy to call async APIs in JS now using Asyncify, see

https://emscripten.org/docs/porting/asyncify.html#making-async-web-apis-behave-as-if-they-were-synchronous

Also see #9041 which is adding an example of returning a value.

It is more work to make C stdin be async, since fread etc. are synchronous, which is what I think added a lot of complexity to this PR. If the C reads are replaced with calls to custom JS functions, then they can be async as in those Asyncify examples.

If it would be helpful, we can add support for Module.stdin returning a promise, and making that work with a new emscripten_read_stdin would be easy, while it would error when called using normal fread.

curiousdannii · 2020-08-02T11:07:00Z

@kripken Just checking what you mean there: it would be fairly easy to add a new C func (emscripten_read_stdin) but lots of work to make the standard fread and the other standard read functions async?

I have code that makes a lot of calls to getc(stdin). Would it be possible to make device that would let these getc calls become async?

Or I've seen that you can provide an stdin function with FS.init, but if you have a lot of data to stream in won't that be horribly inefficient, sending just one character at a time? I'd expect a system where you could give a buffer. But I'm very new to all of this site of things, so I don't have much idea what I'm doing sorry!

Edit: I've tried using FS.init to set a stdin handler, passing in values one at a time, but if you run out and send a null then Emscripten will give getc an EOF.

I've found someone whose done something similar, but it involves hacking the Emscripten generated file quite a lot to add generators/yields: http://mansoft.nl/emscripten/terminal.html

I've found a solution that seems to work for my usecase:

Use the linker's wrap option to wrap getc, redirecting to a custom function if the file is stdin, or calling the real getc otherwise.
Make that custom function be a ASYNCIFY async function. Pause if there isn't any waiting data, otherwise return the next character.

It's not pretty, but it's working for now :).

kripken · 2020-08-03T20:40:33Z

@curiousdannii

Just checking what you mean there: it would be fairly easy to add a new C func (emscripten_read_stdin) but lots of work to make the standard fread and the other standard read functions async?

Yes, exactly.

What is "the linker's wrap option"?

curiousdannii · 2020-08-03T23:36:41Z

Passing in -Wl,--wrap=getc to the compiler, as described here:
https://stackoverflow.com/a/46446749/2854284

This seems to be working quite reliably. But it would only be an option when you're porting something that's quite controlled in how it reads from stdin. And some of the other functions might be harder to wrap than getc. In my case I only needed to wrap getc and ungetc.

I'm also planning to make it more efficient by adding a custom buffer so that it doesn't have to cross the WASM/JS interface each and every call to getc.

Edit: I finished adding the buffer. In case this helps anyone else, here's the code: https://github.com/curiousdannii/emglken/blob/master/src/getc.c

kripken · 2020-08-04T16:09:04Z

Interesting, thanks @curiousdannii , I wasn't aware of that linker option!

albertjan changed the title ~~Async read so you can asynchronously~~ makes FS.read async, so you can read asynchronously from stdin Aug 26, 2015

albertjan force-pushed the async-read branch from 92ec965 to f38b22a Compare September 7, 2015 19:47

albertjan force-pushed the async-read branch from 8a95756 to 348bf63 Compare September 8, 2015 17:34

kripken reviewed Sep 10, 2015
View reviewed changes

albertjan force-pushed the async-read branch 5 times, most recently from c79552b to 7ddc2a2 Compare September 17, 2015 20:14

kripken reviewed Sep 21, 2015
View reviewed changes

kripken reviewed Sep 23, 2015
View reviewed changes

juj added the filesystem label Sep 24, 2015

albertjan added 5 commits October 11, 2015 13:06

add me to the AUTHORS file

ec94f85

async fs read

86f48ab

add test for async read from stdin

975189d

implements 333 and 180

0fb3ca8

add offset and zero back in

b08e375

albertjan force-pushed the async-read branch from e0a685d to 1eaada6 Compare October 11, 2015 11:10

remove duplication

5896b19

albertjan force-pushed the async-read branch from 256b5e9 to 5896b19 Compare October 11, 2015 12:01

kripken mentioned this pull request Feb 24, 2016

Allowing asynchronous 'stdin' streams via Emterpreter #4124

Closed

juj force-pushed the incoming branch from e1622af to d54eb4e Compare May 21, 2018 13:46

CodeFetch mentioned this pull request Nov 30, 2018

Write output to Blob Kagami/ffmpeg.js#54

Closed

kripken closed this Dec 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

makes FS.read async, so you can read asynchronously from stdin #3714

makes FS.read async, so you can read asynchronously from stdin #3714

albertjan commented Aug 25, 2015

kripken commented Aug 27, 2015

albertjan commented Aug 28, 2015

kripken commented Sep 3, 2015

albertjan commented Sep 4, 2015

albertjan commented Sep 4, 2015

kripken commented Sep 4, 2015

albertjan commented Sep 7, 2015

albertjan commented Sep 7, 2015

kripken Sep 10, 2015

albertjan commented Sep 10, 2015

albertjan commented Sep 17, 2015

kripken Sep 21, 2015

kripken commented Sep 21, 2015

albertjan commented Sep 21, 2015

kripken Sep 23, 2015

albertjan Sep 23, 2015

kripken commented Oct 7, 2015

albertjan commented Oct 8, 2015

kripken commented Oct 8, 2015

albertjan commented Oct 8, 2015

albertjan commented Oct 11, 2015

kripken commented Dec 7, 2018

zone117x commented Jul 20, 2019

kripken commented Jul 21, 2019

curiousdannii commented Aug 2, 2020 •

edited

Loading

kripken commented Aug 3, 2020

curiousdannii commented Aug 3, 2020 •

edited

Loading

kripken commented Aug 4, 2020

makes FS.read async, so you can read asynchronously from stdin #3714

makes FS.read async, so you can read asynchronously from stdin #3714

Conversation

albertjan commented Aug 25, 2015

kripken commented Aug 27, 2015

albertjan commented Aug 28, 2015

kripken commented Sep 3, 2015

albertjan commented Sep 4, 2015

albertjan commented Sep 4, 2015

kripken commented Sep 4, 2015

albertjan commented Sep 7, 2015

albertjan commented Sep 7, 2015

kripken Sep 10, 2015

Choose a reason for hiding this comment

albertjan commented Sep 10, 2015

albertjan commented Sep 17, 2015

kripken Sep 21, 2015

Choose a reason for hiding this comment

kripken commented Sep 21, 2015

albertjan commented Sep 21, 2015

kripken Sep 23, 2015

Choose a reason for hiding this comment

albertjan Sep 23, 2015

Choose a reason for hiding this comment

kripken commented Oct 7, 2015

albertjan commented Oct 8, 2015

kripken commented Oct 8, 2015

albertjan commented Oct 8, 2015

albertjan commented Oct 11, 2015

kripken commented Dec 7, 2018

zone117x commented Jul 20, 2019

kripken commented Jul 21, 2019

curiousdannii commented Aug 2, 2020 • edited Loading

kripken commented Aug 3, 2020

curiousdannii commented Aug 3, 2020 • edited Loading

kripken commented Aug 4, 2020

curiousdannii commented Aug 2, 2020 •

edited

Loading

curiousdannii commented Aug 3, 2020 •

edited

Loading