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

Implement bulk re-exports for instantiate setter perf #1957

Closed
wants to merge 2 commits into from

Conversation

guybedford
Copy link
Contributor

Aurelia had some issues with bulk exports causing performance bottlenecks due to the repeated bindings being called in quadratic multiples of the depth of export * usage. This function flattens the binding call in one go. It may be worth always having $__export({ name: 'value' }) as an object so we don't need to overload the function, so could look at a PR to do this if you think it is worthwhile, but the named form of $__export(name, 'value') will still be necessary for backwards compatibility for a while regardless.

Associated ModuleLoader PR is at ModuleLoader/es-module-loader#387.

@@ -349,12 +349,14 @@ export class InstantiateModuleTransformer extends ModuleTransformer {

// then do export bindings of re-exported dependencies
if (externalExportBindings) {
let reexports = {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about __proto__ etc? You probably need to use Object.create(null) or new StringMap().

@arv
Copy link
Collaborator

arv commented Jun 10, 2015

Conceptually this looks good.

@guybedford
Copy link
Contributor Author

Thanks, I've updated the PR.

@arv
Copy link
Collaborator

arv commented Jun 11, 2015

LGTM, I'll squash and merge

@arv
Copy link
Collaborator

arv commented Jun 11, 2015

Committed as 84956a2

@arv arv closed this Jun 11, 2015
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.

2 participants