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

Refactor snapshot build #2825

Merged
merged 30 commits into from
Sep 2, 2019
Merged

Refactor snapshot build #2825

merged 30 commits into from
Sep 2, 2019

Conversation

ry
Copy link
Member

@ry ry commented Aug 28, 2019

Depends on #2827

Fixes #2496
Fixes #2711
Fixes #459
Towards #2608
Towards #986

Prototype: https://github.com/ry/deno_typescript

before (master, 65fa2b8) after
build.py deno; from scratch 2m46.347s 2m28.869s
cargo build; from scratch 6m 35s 4m 14s
build.py deno; touch js/util.ts 1m25.620s 0m24.117s
cargo build; touch js/util.ts BROKEN 43.68s

This patch removes the build dependency on Node and rollup and consequently deletes approximately 1 million lines from third_party/node_modules.

A Rust Crate was published with public V8 snapshotting API https://crates.io/crates/deno_typescript

@ry ry changed the title Integrates deno_typescript crate WIP Integrates deno_typescript crate Aug 28, 2019
@ry ry force-pushed the deno_typescript branch 2 times, most recently from c92e9a2 to 473c07f Compare August 30, 2019 19:19
@ry ry changed the title WIP Integrates deno_typescript crate Refactor snapshot build Aug 30, 2019
Instead of using core/snapshot_creator.rs, instead two crates are
introduced which allow building the snapshot during build.rs.

Rollup is removed and replaced with our own bundler. This removes
the Node build dependency. Modules in //js now use Deno-style imports
with file extensions, rather than Node style extensionless imports.

This improve incremental build time when changes are made //js files by
about 40 seconds.
@ry ry force-pushed the deno_typescript branch from 473c07f to 3a54c03 Compare August 30, 2019 19:24
@ry ry requested a review from piscisaureus August 30, 2019 22:17
@ry ry force-pushed the deno_typescript branch from 04f3f64 to 988370c Compare August 30, 2019 22:33
@ry ry force-pushed the deno_typescript branch from 988370c to 697139b Compare August 30, 2019 22:34
if !build.check_only {
build.run("cli:deno_deps");
}
}
Copy link
Member Author

@ry ry Aug 31, 2019

Choose a reason for hiding this comment

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

Note that cli/build.rs is removed. That means it does not depend on GN during "cargo build"

deps = [
":compiler_bundle",
]
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Note existing snapshot infrastructure is removed.

@@ -1,231 +0,0 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that rollup is removed.

@ry ry force-pushed the deno_typescript branch from 316e46a to eef8854 Compare August 31, 2019 02:12
deno_typescript/ops.rs Outdated Show resolved Hide resolved
deno_typescript/ops.rs Outdated Show resolved Hide resolved
deno_typescript/ops.rs Outdated Show resolved Hide resolved
deno_typescript/ops.rs Outdated Show resolved Hide resolved
deno_typescript/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@piscisaureus piscisaureus left a comment

Choose a reason for hiding this comment

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

LGTM 🌛

@ry ry merged commit d43b43c into denoland:master Sep 2, 2019
@ry
Copy link
Member Author

ry commented Sep 2, 2019

This patch reduced executable size by 7mb. It had some effect on some of the benchmark's memory usage as well.

Screen Shot 2019-09-02 at 6 11 56 PM

@@ -18,9 +18,9 @@ export interface BuildInfo {
export const build: BuildInfo = {
// These string will be replaced by rollup
Copy link

Choose a reason for hiding this comment

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

comment about rollup will be outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants