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

Organize dispatch a bit #2796

Merged
merged 2 commits into from Aug 22, 2019

Conversation

@ry
Copy link
Collaborator

commented Aug 20, 2019

Just some clean up reorganization around flatbuffer/minimal dispatch code. This is prep for adding a JSON dispatcher.

@ry ry changed the title Dispatch flatbuffers Organize dispatch a bit Aug 20, 2019

@ry ry force-pushed the ry:dispatch_flatbuffers branch 2 times, most recently from 0621bc8 to bdd5995 Aug 20, 2019

@ry ry requested a review from piscisaureus Aug 20, 2019

cli/ops/mod.rs Outdated Show resolved Hide resolved

@ry ry force-pushed the ry:dispatch_flatbuffers branch from 2e1f598 to 4299dc5 Aug 21, 2019

@ry ry force-pushed the ry:dispatch_flatbuffers branch from 19facab to 8a24b9c Aug 21, 2019

Organize op dispatch a bit
Just some clean up reorganization around flatbuffer/minimal dispatch
code. This is prep for adding a JSON dispatcher.

@ry ry force-pushed the ry:dispatch_flatbuffers branch from b1af552 to d46ddb1 Aug 21, 2019

@piscisaureus
Copy link
Collaborator

left a comment

This patch is too big to review.
Rubberstamp-only LGTM

@ry ry merged commit bdc97b3 into denoland:master Aug 22, 2019

3 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@ry ry deleted the ry:dispatch_flatbuffers branch Aug 22, 2019

@ry

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 22, 2019

This patch caused the compiler.js bundle to increase from 7.8 MB to 9.1 MB, which ultimately resulted in adding 2.5 MB to the final executable. This is bad and unexpected.

Screen Shot 2019-08-22 at 12 30 43 PM

That said I don't want to revert it because it is preliminary work necessary for ultimately removing flatbuffers - which I think will have a very positive impact on the bundles and executable size.

import * as msg from "gen/cli/msg_generated";
import * as util from "./util";
import { OP_FLATBUFFER } from "./dispatch";
export { msg, flatbuffers };

This comment has been minimized.

Copy link
@ry

ry Aug 22, 2019

Author Collaborator

I suspect the bundle size increase is a result of this line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.