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

[WIP] wasm support #2113

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion package.json
Expand Up @@ -53,7 +53,9 @@
"homepage": "https://github.com/rollup/rollup",
"dependencies": {
"@types/estree": "0.0.38",
"@types/node": "*"
"@types/node": "*",
"@webassemblyjs/ast": "^1.2.2",
"@webassemblyjs/wasm-parser": "^1.2.2"
},
"devDependencies": {
"acorn": "^5.5.3",
Expand Down
3 changes: 3 additions & 0 deletions src/Chunk.ts
Expand Up @@ -11,6 +11,7 @@ import { normalize, resolve, extname, dirname, relative, basename } from './util
import { RawSourceMap } from 'source-map';
import Graph from './Graph';
import ExternalModule from './ExternalModule';
import WasmModule from './WasmModule';
Copy link
Contributor

Choose a reason for hiding this comment

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

I've been thinking perhaps we need to create WasmChunk.ts as well as Chunk is effectively designed to mimic the output graph, so having the two source types would be the best model to match.... this will likely have some complexities on the types like with Module, but I tend to think those are the problems we should be solving here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then as well, just like with Module, an abstract Chunk class would likely make sense to share between both implementations, ideally providing the same sort of interface for src/rollup.index.ts as far as possible, although for the actual render, the output value would be a different interface representing the binary which could be split handled at that point.

import { isExportDefaultVariable } from './ast/variables/ExportDefaultVariable';
import Variable from './ast/variables/Variable';
import NamespaceVariable from './ast/variables/NamespaceVariable';
Expand Down Expand Up @@ -478,6 +479,8 @@ export default class Chunk {
} else if (resolution instanceof ExternalModule) {
node.renderFinalResolution(code, `"${resolution.id}"`);
// AST Node -> source replacement
} else if (resolution instanceof WasmModule) {
node.renderFinalResolution(code, `"${resolution.id}"`);
Copy link
Author

Choose a reason for hiding this comment

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

Currently the generated code is

Promise.resolve(require("/foobar/sventest/addTwo/src/addTwo.wasm")).then(({addTwo}) => {
  console.log("addTwo(1, 2)", addTwo(1, 2));
});


	function then(resolve) {
	  // ...
	}


var addTwo = /*#__PURE__*/Object.freeze({
  then: then
});

I believe I need to change the resolution to return the addTwo object.

Copy link
Contributor

Choose a reason for hiding this comment

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

The inlining decision for dynamic imports is made here - https://github.com/rollup/rollup/pull/2113/files#diff-8c6f11349cc1bb370bdd557ed2bcbe7dR443. Can that be extended to a WASM check?

Copy link
Author

Choose a reason for hiding this comment

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

Ok, this has been fixed.

} else {
node.renderFinalResolution(code, resolution);
}
Expand Down
53 changes: 41 additions & 12 deletions src/Graph.ts
Expand Up @@ -3,14 +3,15 @@ import injectDynamicImportPlugin from 'acorn-dynamic-import/lib/inject';
import { timeEnd, timeStart } from './utils/timers';
import first from './utils/first';
import Module from './Module';
import WasmModule from './WasmModule';
import ExternalModule from './ExternalModule';
import ensureArray from './utils/ensureArray';
import { handleMissingExport, load, makeOnwarn, resolveId } from './utils/defaults';
import { mapSequence } from './utils/promise';
import transform from './utils/transform';
import relativeId, { nameWithoutExtension } from './utils/relativeId';
import error from './utils/error';
import { isRelative, resolve, basename, relative } from './utils/path';
import { isRelative, resolve, basename, relative, extname } from './utils/path';
import {
InputOptions,
IsExternalHook,
Expand Down Expand Up @@ -52,7 +53,7 @@ export default class Graph {
importedModule: string,
importerStart?: number
) => void;
moduleById: Map<string, Module | ExternalModule>;
moduleById: Map<string, Module | WasmModule | ExternalModule>;
Copy link
Contributor

Choose a reason for hiding this comment

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

A base-level Module abstract class may well make sense at this point I think.

modules: Module[];
onwarn: WarningHandler;
plugins: Plugin[];
Expand Down Expand Up @@ -425,9 +426,9 @@ export default class Graph {
timeStart('generate chunks', 2);

// TODO: there is one special edge case unhandled here and that is that any module
// exposed as an unresolvable export * (to a graph external export *,
// either as a namespace import reexported or top-level export *)
// should be made to be its own entry point module before chunking
// exposed as an unresolvable export * (to a graph external export *,
// either as a namespace import reexported or top-level export *)
// should be made to be its own entry point module before chunking
const chunkList: Chunk[] = [];
if (!preserveModules) {
const chunkModules: { [entryHashSum: string]: Module[] } = {};
Expand Down Expand Up @@ -497,14 +498,14 @@ export default class Graph {
let curEntry: Module, curEntryHash: Uint8Array;
const allSeen: { [id: string]: boolean } = {};

const orderedModules: Module[] = [];
const orderedModules: Module | WasmModule[] = [];

const dynamicImports: Module[] = [];
const dynamicImports: Module | WasmModule[] = [];
const dynamicImportAliases: string[] = [];

let parents: { [id: string]: string };

const visit = (module: Module) => {
const visit = (module: Module | WasmModule) => {
// Track entry point graph colouring by tracing all modules loaded by a given
// entry point and colouring those modules by the hash of its id. Colours are mixed as
// hash xors, providing the unique colouring of the graph into unique hash chunks.
Expand All @@ -523,6 +524,7 @@ export default class Graph {

for (let depModule of module.dependencies) {
if (depModule instanceof ExternalModule) continue;
if (depModule instanceof WasmModule) continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we should not permit WasmModule dependencies at all initially, and throw before even populating dependencies with a WasmModule?


if (depModule.id in parents) {
if (!allSeen[depModule.id]) {
Expand All @@ -537,7 +539,10 @@ export default class Graph {

if (this.dynamicImport) {
for (let dynamicModule of module.dynamicImportResolutions) {
if (dynamicModule.resolution instanceof Module) {
if (
dynamicModule.resolution instanceof Module ||
dynamicModule.resolution instanceof WasmModule
) {
if (dynamicImports.indexOf(dynamicModule.resolution) === -1) {
dynamicImports.push(dynamicModule.resolution);
dynamicImportAliases.push(dynamicModule.alias);
Expand Down Expand Up @@ -611,12 +616,35 @@ Try defining "${chunkName}" first in the manualChunks definitions of the Rollup
});
}

private fetchModule(id: string, importer: string): Promise<Module> {
private fetchModule(id: string, importer: string): Promise<Module | WasmModule> {
// short-circuit cycles
const existingModule = this.moduleById.get(id);
if (existingModule) {
if (existingModule.isExternal) throw new Error(`Cannot fetch external module ${id}`);
return Promise.resolve(<Module>existingModule);
return Promise.resolve(<Module | WasmModule>existingModule);
}

if (extname(id) === '.wasm') {
const module = new WasmModule(this, id);
this.moduleById.set(id, module);

// FIXME(sven): alternatively we can use a loadBinary method
return this.load(id)
.catch((err: Error) => {
// FIXME(sven): error handling here
throw err;
})
.then((data: string) => {
// TODO(sven): still check magic header here
const bin = new Buffer(data);
module.setSource(bin);

this.fetchAllDependencies(module);

this.modules.push(module);

return module;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't worry about having this blatant hack here - let's leave this as a big TODO and come back to it later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking about this a little more - instead of trying to introduce some new type of plugin or resolve API to distinguish WASM, let's just use the binary header bytes detection method for this. Saves a lot of pain!

}

const module: Module = new Module(this, id);
Expand Down Expand Up @@ -705,7 +733,7 @@ Try defining "${chunkName}" first in the manualChunks definitions of the Rollup
});
}

private fetchAllDependencies(module: Module) {
private fetchAllDependencies(module: Module | WasmModule) {
// resolve and fetch dynamic imports where possible
const fetchDynamicImportsPromise = !this.dynamicImport
? Promise.resolve()
Expand All @@ -721,6 +749,7 @@ Try defining "${chunkName}" first in the manualChunks definitions of the Rollup
};
return;
}

const alias = nameWithoutExtension(basename(replacement));
if (typeof dynamicImportExpression !== 'string') {
module.dynamicImportResolutions[index] = { alias, resolution: replacement };
Expand Down
2 changes: 1 addition & 1 deletion src/Module.ts
Expand Up @@ -104,7 +104,7 @@ export default class Module {
code: string;
comments: CommentDescription[];
context: string;
dependencies: (Module | ExternalModule)[];
dependencies: (Module | ExternalModule | WasmModule)[];
excludeFromSourcemap: boolean;
exports: { [name: string]: ExportDescription };
exportsAll: { [name: string]: string };
Expand Down
193 changes: 193 additions & 0 deletions src/WasmModule.ts
@@ -0,0 +1,193 @@
import { decode } from '@webassemblyjs/wasm-parser';
import ast from '@webassemblyjs/ast';

import Graph from './Graph';
import { IdMap, ModuleJSON, WebAssemblyJSAst } from './rollup/types';

import { makeLegal } from './utils/identifierHelpers';
import { RenderOptions } from './utils/renderHelpers';
import { basename, extname } from './utils/path';

import ExternalModule from './ExternalModule';
import Module from './Module';

import ModuleScope from './ast/scopes/ModuleScope';
import Variable from './ast/variables/Variable';
import NamespaceVariable from './ast/variables/NamespaceVariable';

const decoderOpts = {
ignoreCodeSection: true,
ignoreDataSection: true
};

const buildLoader = ({ URL, IMPORT_OBJECT }) => `
function then(resolve) {
if (typeof WebAssembly.instantiateStreaming !== 'function') {
throw new Error('WebAssembly.instantiateStreaming is not supported');
}

if (typeof window.fetch !== 'function') {
throw new Error('window.fetch is not supported');
}

const req = window.fetch('${URL}');

WebAssembly
.instantiateStreaming(req, ${IMPORT_OBJECT || '{}'})
.then(res => res.instance.exports)
.then(resolve)
.catch(resolve);
}
`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be treated as the "dynamic import mechanism" as is used in the JS Chunk.ts file that does the import to the associated wasmChunk.ts file?

Copy link
Contributor

Choose a reason for hiding this comment

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

(and let me know if that would handle the issue you raised now?)

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it would need to be a wasmImport or similar mechanism associated with the dynamic import specifically in the wasm case.

The point being that we are handling this in the src/ast/nodes/Import.ts render itself as the dynamic import rendering.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry but i'm really not sure to understand that.

"dynamic import mechanism"

Yes, that's the code behind import.

(and let me know if that would handle the issue you raised now?)

You mean the binding conflict issue?


export interface ExportDescription {
localName: string;
identifier?: string;
}

export interface ImportDescription {
source: string;
name: string;
module: Module | ExternalModule | null;
}

export default class WasmModule {
id: string;
graph: Graph;
code: Buffer;

scope: ModuleScope;

ast: WebAssemblyJSAst.Program;
Copy link
Author

Choose a reason for hiding this comment

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

We could provide the same AST manipulation logic to plugins as with JS.

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome!

Copy link
Author

@xtuc xtuc Apr 6, 2018

Choose a reason for hiding this comment

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

Forgot to mention that we can enable AST diffing + wasm module manipulation using this API. Which could be useful for plugin (mutable global import polyfilling for ex 💯 ).


sources: string[];
resolvedIds: IdMap;

dynamicImportResolutions: {
alias: string;
resolution: Module | ExternalModule | string | void;
}[];
dependencies: (Module | ExternalModule | WasmModule)[];

imports: { [name: string]: ImportDescription };
exports: { [name: string]: ExportDescription };

// this is unused on Module,
// only used for namespace and then ExternalExport.declarations
declarations: {
'*'?: NamespaceVariable;
[name: string]: Variable | undefined;
};

isExternal: false;

constructor(graph: Graph, id: string) {
this.id = id;
this.graph = graph;
this.code = new Buffer('');

this.ast = null;

// imports and exports, indexed by local name
this.imports = Object.create(null);
this.exports = Object.create(null);
this.resolvedIds = Object.create(null);
this.declarations = Object.create(null);

// all dependencies
this.dynamicImportResolutions = [];
this.sources = [];
this.dependencies = [];

this.scope = new ModuleScope(this);
Copy link
Author

Choose a reason for hiding this comment

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

I would like to remove the scope but a lot of code depends on it.

Copy link
Member

Choose a reason for hiding this comment

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

As far as I can see the only outside usages of .scope are in Chunk.setIdentifierRenderResolutions() which is basically the deconflicting logic for the given chunk. Instead of adding the scope here, we could introduce a local variable const orderedJsModules = this.orderedModules.filter(module => module instanceOf Module) and use it in Chunk.setIdentifierRenderResolutions() to only deconflict JS modules. But I may be overlooking other usages here.


// expose Thenable
this.exports.then = {
localName: 'then'
};
}

render(options: RenderOptions) {
const URL = 'foo';
const content = buildLoader({ URL });

return { trim() {}, content };
Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure how to constructor the MagicString object in that case.

Copy link
Contributor

Choose a reason for hiding this comment

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

WasmModule.render must return a Buffer surely? I think we should handle the distinction in the caller sites.

Copy link
Author

@xtuc xtuc Apr 6, 2018

Choose a reason for hiding this comment

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

Following my comment #2113 (comment), since i'm writing the file myself I bypass this logic actually, but I use it to inject the loader needed on the JS module. Does it make sense to you?

}
Copy link
Contributor

Choose a reason for hiding this comment

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

If we have WasmChunk calling render, then it will be ok for the render method to return a binary (plus binary source map in due course).


getDynamicImportExpressions(): (string | Node)[] {
// FIXME(sven): consider ModuleImport as dynamicImports?
return [];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It won't be possible for WASM to dynamically import another module, so these things all can then be removed.

Copy link
Author

@xtuc xtuc Apr 13, 2018

Choose a reason for hiding this comment

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

I agree, it was more an implementation detail about how to declare the imports. The static imports are better.


markExports() {}

// TODO(sven): what is this?
namespace(): NamespaceVariable {
if (!this.declarations['*']) {
this.declarations['*'] = new NamespaceVariable(this);
}

return this.declarations['*'];
}

basename() {
const base = basename(this.id);
const ext = extname(this.id);

return makeLegal(ext ? base.slice(0, -ext.length) : base);
}

getExports() {
return Object.keys(this.exports);
}

getReexports() {
return [];
}

includeInBundle() {
return false;
}

linkDependencies() {
const { imports, exports } = this;

ast.traverse(this.ast, {
// ModuleImport({node}: any) {
// const source = node.module
// const name = node.name;
// imports[`${source}.${name}`] = { source, name, module: null };
// },
// ModuleExport({node}: any) {
// const name = node.name;
// exports[name] = {
// localName: name
// };
// }
});
}

bindReferences() {}

toJSON(): ModuleJSON {
return {
id: this.id,
dependencies: this.dependencies.map(module => module.id),
code: this.code,
originalCode: '',
originalSourcemap: undefined,
ast: this.ast,
sourcemapChain: null,
resolvedIds: this.resolvedIds
};
}

traceExport(name: string): Variable {
return new Variable(name);
}

setSource(bin: Buffer) {
this.code = bin;
this.ast = decode(bin, decoderOpts);
}
}
8 changes: 7 additions & 1 deletion src/rollup/index.ts
@@ -1,5 +1,5 @@
import { getTimings, initialiseTimers, timeEnd, timeStart } from '../utils/timers';
import { basename } from '../utils/path';
import { basename, extname } from '../utils/path';
import { writeFile } from '../utils/fs';
import { mapSequence } from '../utils/promise';
import error from '../utils/error';
Expand Down Expand Up @@ -238,6 +238,12 @@ export default function rollup(
code += `//# ${SOURCEMAPPING_URL}=${url}\n`;
}

const wasmModule = modules.find(x => extname(x.id) === '.wasm');

if (wasmModule !== undefined) {
promises.push(writeFile('./dist/addTwo.wasm', wasmModule.code));
}
Copy link
Author

Choose a reason for hiding this comment

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

I have no idea how to emit the file correct since it's a single file build.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is why I was wondering if it's worth considering each chunk (file) as having a WASM and JS component, where if the WASM or JS component is empty it is just a WASM or JS file and not both. So Chunk as consisting of both WASM and JS modules, where the render process for a chunk then returns both a magic string and a buffer, and if either is empty it's a WASM or JS file. But perhaps that is quite odd.

The alternative would be a more generic emission API, or to just do a specific check for a WASM output.

Copy link
Author

Choose a reason for hiding this comment

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

I would prefer a more generic emission API. I feel like it's more future proof and we should take advantage of the Graph (to handle spllited or nested wasm builds).

Note that I have to MagicString in the initial JS module and write the wasm module in a file.


promises.push(writeFile(file, code));
return (
Promise.all(promises)
Expand Down