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

perf(compiler): skip type check and emit in bazel in some cases. #19646

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@tbosch
Copy link
Member

tbosch commented Oct 10, 2017

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

Other information

@googlebot googlebot added the cla: yes label Oct 10, 2017

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Oct 10, 2017

import * as tsickle from 'tsickle';
import * as ts from 'typescript';

interface EmitCachEntry {

This comment has been minimized.

Copy link
@gkalpak

gkalpak Oct 10, 2017

Member

EmitCachEntry --> EmitCacheEntry

@tbosch tbosch force-pushed the tbosch:g3_perf branch 2 times, most recently from 8515186 to 4a272ba Oct 10, 2017

@tbosch tbosch requested a review from chuckjaz Oct 11, 2017

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Oct 11, 2017

@chuckjaz
Copy link
Member

chuckjaz left a comment

Some nits to consider.

if (cacheEntry) {
let useEmitCache = false;
if (targetGeneratedFile && !program.hasChanged(targetSourceFile.fileName)) {
// we emitted a GeneratedFile with the same content before -> use the cache

This comment has been minimized.

Copy link
@chuckjaz

chuckjaz Oct 11, 2017

Member

nit: /same content before/same content as before/

This comment has been minimized.

Copy link
@tbosch

tbosch Oct 11, 2017

Author Member

Done

export interface EmitCallback { (args: EmitArguments): ts.EmitResult; }

/**
* Represents a not yet emitted generated file.

This comment has been minimized.

Copy link
@chuckjaz

chuckjaz Oct 11, 2017

Member

Consider "Represents a generated file that has not yet been emitted".

This comment has been minimized.

Copy link
@tbosch

tbosch Oct 11, 2017

Author Member

Done

srcFileName: string;
/**
* The file name of the generated but not yet emitted file.
* I.e. this ends with `.ts`.

This comment has been minimized.

Copy link
@chuckjaz

chuckjaz Oct 11, 2017

Member

I don't understand what this i.e. is an example of?

This comment has been minimized.

Copy link
@tbosch

tbosch Oct 11, 2017

Author Member

Removed this.

@@ -12,3 +12,4 @@ import {CompilerHost, CompilerOptions, Program} from './api';

export {createCompilerHost} from './compiler_host';
export {createProgram} from './program';
export {isGeneratedFile} from './util';

This comment has been minimized.

Copy link
@chuckjaz

chuckjaz Oct 11, 2017

Member

Consider restoring the \n at the end of the file.

This comment has been minimized.

Copy link
@tbosch

tbosch Oct 11, 2017

Author Member

Done


export function isGeneratedFile(fileName: string): boolean {
return GENERATED_FILES.test(fileName);
}

This comment has been minimized.

Copy link
@chuckjaz

chuckjaz Oct 11, 2017

Member

Consider restoring \n at the end of the file.

This comment has been minimized.

Copy link
@tbosch

tbosch Oct 11, 2017

Author Member

Done

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Oct 11, 2017

perf(compiler): skip type check and emit in bazel in some cases.
If no user files changed:
- only type check the changed generated files

Never emit non changed generated files
- we still calculate them, but don’t send them through
  TypeScript to emit them but cache the written files instead.

@tbosch tbosch force-pushed the tbosch:g3_perf branch from 678f485 to c1f6b13 Oct 11, 2017

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Oct 11, 2017

@chuckjaz chuckjaz added this to the Merge-queue milestone Oct 11, 2017

@chuckjaz chuckjaz closed this in a22121d Oct 11, 2017

chuckjaz added a commit that referenced this pull request Oct 12, 2017

chuckjaz added a commit that referenced this pull request Oct 12, 2017

tbosch added a commit that referenced this pull request Oct 12, 2017

@tbosch tbosch deleted the tbosch:g3_perf branch Nov 1, 2017

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