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

Incorrect example of browser/node.js library compatibility #256

Closed
trevnorris opened this issue Jul 7, 2015 · 33 comments
Closed

Incorrect example of browser/node.js library compatibility #256

trevnorris opened this issue Jul 7, 2015 · 33 comments

Comments

@trevnorris
Copy link

There's a problematic example from the following in NonWeb.md:

A symmetric example in JavaScript would be the Loader spec, intended to be implemented by both browsers and node.js.

Using the Loader spec in node.js has practical issues. Mainly because it specifies that all loading of imports must be done asynchronously. I've outlined a few issues and limitations of using asynchronous module loading in node.js:

  • All modules exist locally (i.e. the hard drive) and their retrieval does not suffer from latency, round-trip or other browser related I/O issues.
  • All imports will be resolved and loaded at the beginning of the application. Preventing the application from starting until all modules have been loaded. Basically even asynchronous loading will still block the application.
  • The number of modules that can be asynchronously retrieved is limited by both the number of leafs in the dependency graph and the number of threads in libuv's thread pool. Further inhibiting possible performance gains from simultaneous retrieval.
  • The asynchronous nature of Loader cannot be used safely with the existing synchronous require(). For example, a synchronously loaded module may cause side affects that impact the asynchronously loaded module.
  • The import syntax in general is limiting to node.js module loading. Specifically pertaining to the loading of native modules that decide at run-time what build to use (e.g. debug/release).

Basically, implementing Loader in node.js has little to zero likelihood of a performance advantage and has high potential to cause non-trivial breakage in the ecosystem.

I've been considering what similar implementations would work well on browser/node.js and to be honest I can't think of one. Starting in v3 we use a bastardized Uint8Array() as the backing store for Buffer to get around performance issues. All of our I/O bindings sit very close to the syscalls underneath and support almost anything you could do at that level.

In short, any browser/node.js compatibility I've been able to think of may be useful for module authors who want to use the same code on both, but it's not useful for the core code.

@bmeck
Copy link

bmeck commented Jul 7, 2015

Might want to note as well: standard feature detection of polyfills uses variables / conditional branching to figure out if a polyfill is required to be loaded, current ecmascript loader syntax does not support this.

@lukewagner
Copy link
Member

@trevnorris IIUC, you're saying that there may actually not be an intention for the Loader spec to be implemented in node and so the example should be removed. If so, feel free to file a PR to remove.

@bmeck Conditional importing (at least for builtin modules) for the purpose of polyfilling is indeed something I've been thinking we need. This is missing in the docs and I've been meaning to write a PR. With dynamic linking, apps will have full power so technically it's not necessary, but load-time does allow some performance advantages (especially for builtins).

@trevnorris
Copy link
Author

@lukewagner Removal of the Loader example was the main reason for this issue. I will create a PR.

Addressing a more general point, I don't believe it's necessary for the document to state how shared specs could be proposed. Unless it ships with the VM, implementation isn't guaranteed. And to-date there hasn't been one non-ecma API that's been fully implemented in core. Module authors might do it, but generally browser APIs are too restrictive for the server case.

Afterwards it talks about "[t]his situation" (I assume it refers to the shared spec) and then references file I/O. I fail to see how an fs API for the browser could have near enough utility on the server. And, at least in node's case, fs operations are already well defined. Again, no overlap.

Could you help me understand "standardizing existing practice"? I can't determine if that means there will be libraries not included by default but sanctioned by wasm or simply community written modules anyone can use.

Short of it is the entire paragraph makes flawed assumptions about how useful browser specific APIs will be on the server. I don't see the need for wasm to attempt addressing this type of cross compatibility.

@lukewagner
Copy link
Member

Re-reading NonWeb.md, I see that it has since grown a new sentence which I think is the source of your consternation here:

In that respect, WebAssembly would err towards standardizing existing practice through libraries, and let developers choose which libraries to use.

@jfbastien, I'm pretty sure we all agreed that we don't want to be standardizing libraries (for anything not core to execution, like sbrk) in WebAssembly; that instead we want to rely on the host environment's APIs. I think this sentence suggests otherwise and we should rewrite.

What the original text said was that the community would be expected to develop libraries that implemented portable abstractions (like POSIX) in terms of host environment APIs.

@trevnorris
Copy link
Author

@lukewagner Ah yeah. That wording, IMO, is more appropriate. I'll let you direct how that wording should change.

@jfbastien
Copy link
Member

We indeed don't intend to standardize libraries, at least not any time soon. I do want to highlight the idea that WebAssembly will provide the basics needed to implement libraries, and then rely on existing practice to figure out what libraries are "standardized" in SDKs. This isn't some mandatory SDK, it's just a group of components that would end up clustering and working well together.

You can choose the SDK you want, the package/dependency manager you want, but the hope is that at some point there are e.g. filesystem APIs that work pretty much the same across wasm embedders.

@trevnorris
Copy link
Author

@jfbastien Has it been discussed which "existing practices" would be used, or how they would be chosen?

In regards to the example filesystem API. Would it be a fairly extensive API that would replicate much of the lower level functionality of the OS?

Also, I've gotten mixed messages about asynchronous functionality within wasm, whether an event loop would be permitted, etc. Can you clarify whether wasm would support an event loop so file system operations could be done asynchronously, or would it require spawning a thread and executing the task synchronously?

Sorry for all the questions. I'm trying to identify the applicable scope of wasm on the server.

@jfbastien
Copy link
Member

@trevnorris people in a smoked filled room would make those decision. I kid, I kid! The idea came about when I described how the C++ standards committee has recently been working w.r.t. standardizing library features. There's no set measurement of what goes in and what doesn't, but in general the agreement is that inventing things isn't the library's business. The goal is rather to standardize existing practice while ensuring that libraries fit a cohesive model.

We'd like something similar for WebAssembly, but I don't think we want libraries explicitly in the standards process. We want developers to create interesting libraries, and other developers can eventually pull "winners" together into something that's cohesive.

This is a very high-level idea, more of a "let's point in this direction and see what happens" type of thing.

On the event loop question: it's complicated. MVP has no threads, it integrates with the embedder, and in the case of the web platform that implies having an event loop whether we want it or not. With threads we can expose primitives that enable epoll / select behavior, and we can do better with signals, pipes, and file descriptor-like features. We've had discussions along these lines in #104, and somewhat related discussions in #126 and #238. I think dynamic linking will make these abstractions much nicer than what MVP will offer.

@trevnorris
Copy link
Author

@jfbastien Thanks. To summarize what I understand: wasm will "sanction" the libraries most adopted by the community, but not incorporate those.

In regards to the event loop, will that be implemented by the interpreter VM? In other words, implemented by whatever is executing the code (V8) or implemented by who is using the code (node.js).

From node's perspective it would be better if we could simply propagate state asynchronously across function calls, and had the hooks to call those functions when needed. There are some things that depend on event loop ordering (e.g. ensuring a callback runs after the cleanup phase) that I doubt wasm wants to define.

@lukewagner
Copy link
Member

I'm not really sure what "sanction" means in a wasm spec context; I understand "specify" (in the sense that the C/C++ specs specify standard libraries) and "not specify" (in the sense that C/C++ do not say anything about OpenGL/DirectX) and I think we're saying the wasm spec will "not specify" anything outside of core execution, at least in the MVP or soon-after-MVP. I can, however, imagine a farther-off world where, to allow super-easy portability between Web and non-Web host environments, the wasm spec starts specifying a growing set of builtin modules that overlap (and are logically implemented in terms of) host environment APIs, but I think we'd want a pretty strong set of use cases before going that route.

@trevnorris
Copy link
Author

@lukewagner By "sanction" I mean that wasm would deem a popular community library as the (sort of) standard. Development of the library continues outside of wasm but there is then additional communication concerning future development, etc.

In node this would be akin to npm (node's package manager) and nan (native addon abstraction). Both of these only use node's public API, and anyone is free to write a similar package, but they already have such significant community support that they're essentially considered part of node.

@trevnorris
Copy link
Author

With this I was addressing the "farther-off" scenario you mentioned. By that time there may be libraries that have enough support, and community share, that it wouldn't make sense for wasm to define its own spec to do the same thing. It would also allow wasm to continue on as the small core it's starting as.

Some consider remaining small a disadvantage, but it has worked very well to node's substantial growth. And as a node core maintainer I can say it's much more pleasant. When I worked at Mozilla I was always impressed with engineers working in moz-central. ;-)

@lukewagner
Copy link
Member

Ah, that makes sense.

@caridy
Copy link

caridy commented Jul 10, 2015

All imports will be resolved and loaded at the beginning of the application. Preventing the application from starting until all modules have been loaded. Basically even asynchronous loading will still block the application.

@trevnorris this is not accurate. lets keep in mind that there are two ways to import modules, using the declarative form, import ... ; and the imperative form, using a loader, and I'm not talking specifically about the loader you have to instantiate, I'm talking about the runtime built-in loader that will be accesible by any module imported in the runtime (think about this as equivalent to require, but async), which allows you to "asynchronously" load any relative module on demand. We have been discussing this for a while as a way to match the laziness aspect of require() calls within functions. There will be a little bit of a refactor hazard since import calls using the imperative form will be "thenable", but I think that aspect will be ok. We can discuss more for sure...

@bmeck
Copy link

bmeck commented Jul 10, 2015

@caridy this is where things get complicated due to a lack of clarity on loader needing to be what backs import. if the WHATWG Loader is the backer of import syntax (there is no requirement in ES2015 for this) we have a problem since this can have a change of ordering if require statements block during async module resolution.

The leaf modules will still be evaluated in order according to Loader, but if a module uses require and blocks it has different semantics in terms of when side effects take place. As such, we need to be careful and I am a bit concerned about putting evaluation order in this respect. require side effects are immediate even though Loader based import sounds like it would be done in a task queue and have to wait.

Using the lazy loading and getting a Promise back is a different topic, and less of a wart. I think the main concern is Loader trying to mandate the specifics of import.

There is also a problem with circular deps if Loader backs import as the spec stands. It would throw, node does not and even has documents on what steps should occur https://nodejs.org/api/all.html#all_cycles .

@bmeck
Copy link

bmeck commented Jul 10, 2015

reference to imperative imports: whatwg/loader#36 , which does seem to go around messing with meta properties and assurance of being the import syntax backer

@trevnorris
Copy link
Author

@caridy My statement is accurate, as it specifically states that usage of import will still prevent the application, regardless of being asynchronous, from starting until all modules have been loaded. So use of import on the server doesn't have any proof of aiding performance, since system resources are constrained anyway, but adds a lot of complexity by comparison to simply synchronously loading.

which allows you to "asynchronously" load any relative module on demand.

I can see the advantage of this in a browser, but not on the server. We're not waiting 200ms for data from a server over TCP. We're waiting 100 microseconds to be read from local disk.

@matthewp
Copy link

@bmeck That's one possible form, there will also be a global loader available somewhere like perhaps global.loader so user code can do:

let _ = await loader.import("underscore");

@bmeck
Copy link

bmeck commented Jul 10, 2015

as stated in whatwg/loader#54 (comment) that is not available at module or top level under the current async/await proposal , also see the other issues

@caridy
Copy link

caridy commented Jul 10, 2015

@trevnorris the current state of the discussion that gravitates about import declarative syntax and import imperative syntax is what I was referencing to. The fact that you might use the import imperative form, which will NOT load the module until the invocation gets executed (which could happen at any given time). Saying that import requires all modules to be loaded, is not true. In the other hand, saying that all modules imported using the import declarative form are required to be loaded, it is certainly true.

@caridy
Copy link

caridy commented Jul 10, 2015

@trevnorris:

Using the lazy loading and getting a Promise back is a different topic, and less of a wart. I think the main concern is Loader trying to mandate the specifics of import.

It is not a different topic, it is the way people will write JS in the future. Just by adding async into the mix will give us a very nice form that it is very clear in intent, and it is easy to read.

There is also a problem with circular deps if Loader backs import as the spec stands. It would throw, node does not and even has documents on what steps should occur

can you elaborate more? what's the problem here?

@bmeck
Copy link

bmeck commented Jul 11, 2015

@caridy

it is the way people will write JS in the future

This is making an assumption about Promises being the way Loader works, once again read issues about timing of side effects. Synchronous looking syntax does not fix this. Please reply to the breaking change and concerns about breakage; unsure if you understand the problem or why a breaking change concerns us.

can you elaborate more? what's the problem here?

Breaking change, see linked reference to node documentation on how cycles are handled above vs http://whatwg.github.io/loader/#reflective-resolve-export

@caridy
Copy link

caridy commented Jul 11, 2015

@bmeck @trevnorris I'm positive that we are not in the same page about loader, let's sort this out. We are committed to make this to solve the node use case, so lets go over the details. The only thing I can tell you at this point is that we have spent considerable amount of time thinking and modeling this is such a way that it "should" work on the node scenario. Do you have time for a quick chat soon?

As for the recurrent question, yes, WHATWG Loader "is" the backer of import declarative and imperative form.

@trevnorris
Copy link
Author

@caridy

We are committed to make this to solve the node use case

I don't believe this. No one has addressed the fact that asynchronously loading modules is completely pointless in node. Using await to emulate the same behavior makes the entire thing pointless and over engineered. Since loading truly synchronously does exactly what we need and is much less complex.

So please, convince me why breaking node's ecosystem and introducing complexity so loads can still work the same way they always have is a good thing.

we have spent considerable amount of time thinking and modeling this is such a way that it "should" work on the node scenario.

This just irks me. I don't remember an invite to the node community for feedback with something even as simple as an open issue. I haven't seen any dialog with any core maintainers, who are the ones reading every issue posted, who are the ones doing engagements speaking and meeting with community members. Think and model all you want, but if it lacks complete practical insight then it probably won't be useful.

@trevnorris
Copy link
Author

@caridy

yes, WHATWG Loader "is" the backer of import declarative and imperative form.

But, AFAIK, anything not strictly ECMA doesn't make it into V8. Hence node still has no need to use it.

@guybedford
Copy link

The loader spec is especially designed to be able to be as backwards-compatible as possible with existing ecosystems. The V8 implementation will come with a single hook for Node - HostResolveImportedModule, to return the Module object for a given unnormalized import name. As long as it is possible to determine the module format of a module ahead of time (CommonJS or ES Module), all CommonJS modules can be loaded synchronously and equivalently using the exact same require code currently in Node via a loader compatibility layer, while enabling ES modules as well. These are the hard problems the authors have been thinking about. When CommonJS modules load ES modules, you just hook back to the ES loader.

In Node, dynamic and declarative imports via require are treated as the same thing and synchronous. On startup, there is neglibible performance difference between ES modules and CommonJS module systems, but the dynamic requires will certainly benefit by not blocking - it's truer to Node's own philosophy to do that.

This on its own is not enough to cover all the conditional cases though as you say. Configuration-based approaches are a good way to handle this - a loader configuration informs that the fs module should map to something else in a different environment, and the module retains its declarative exports without dropping down to dynamic loader imports. Conditional loading becomes conditional map. The exact semantics need to be worked out though, and I've been experimenting a bit with this recently in SystemJS.

@trevnorris
Copy link
Author

@guybedford

using the exact same require code currently in Node via a loader compatibility layer

This says to me it would still use Promises. Is that correct?

dynamic and declarative imports via require are treated as the same thing

I don't understand how this is possible. One is resolved during parsing and the other during execution.

dynamic requires will certainly benefit by not blocking

Honestly I'm getting tired of pointing out this is not true. It takes V8 longer to compile and run the scripts than it does to retrieve them from disk.

it's truer to Node's own philosophy to do that.

This isn't a valid argument of why something should be done that will break the ecosystem. And it's only the philosophy when practical. There are more than a few operations performed synchronously for better performance.

Configuration-based approaches are a good way to handle this

All I read here is more work and more complexity to achieve the same result.

These discussions are revolving around how implementing Loader is technically feasible. First I'd like someone to convince me it's practical. So far all I see is breakage and complexity. Loading async isn't a valid argument. A module can be created easy enough to parse any number of import types. Please, give me a practical valid argument why this is an improvement over what node currently uses.

@guybedford
Copy link

@trevnorris I guess I'm conflating Loader and ES modules somewhat here. The previous points all apply equally well to ES modules without any loader spec. The loader spec comes in because we do need dynamic loading as well, and then need to have a well-defined shared pipeline and registry between the declarative and dynamic loading, which the loader provides.

I was referring to CommonJS that dynamic and declarative imports via require are treated as the same thing - this synchronous loading code (exactly in Node's require currently) can be called from the ES loading hook, as a subloader if that makes sense? The individual requires don't need to be interrupted at all. This is also exactly how the loader spec is designed to integrate with CommonJS.

There shouldn't need to be a break in the ecosystem - I've yet to find a reason for why this might not be possible as a fully backwards-compatible change. I also don't think it would be too hard, but would require some engagement when that V8 ES module hook eventually lands.

@matthewp
Copy link

I don't see why the Loader spec cannot/should not be updated to accommodate environments where synchronous loading makes more sense. Remove the word "Promise" from the spec and the steps should still work the same way. You will still need different syntax for declarative/imperative imports but that is another matter.

@bmeck
Copy link

bmeck commented Jul 11, 2015

@matthewp timing matters, Promises do more than use a data structure they also use a task queue executed after the stack unwinds from the event loop.

@bmeck
Copy link

bmeck commented Jul 13, 2015

@caridy we can setup a time to have a call, as it does sound like we have drastically different views

@caridy
Copy link

caridy commented Jul 13, 2015

@bmeck that will be great, later today, or tomorrow is fine for me, just ping me: caridy at gmail dot com

@lukewagner
Copy link
Member

I filed #266 to make two improvements suggested by this issue.

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

No branches or pull requests

7 participants