Skip to content

Commit 0dc261d

Browse files
committed
Performance improvements in comment emit.
1 parent a1518d3 commit 0dc261d

7 files changed

Lines changed: 322 additions & 352 deletions

File tree

src/compiler/comments.ts

Lines changed: 138 additions & 224 deletions
Large diffs are not rendered by default.

src/compiler/printer.ts

Lines changed: 75 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -161,21 +161,16 @@ const _super = (function (geti, seti) {
161161

162162
const comments = createCommentWriter(host, writer, sourceMap);
163163
const {
164-
getLeadingComments,
165-
getTrailingComments,
166-
getTrailingCommentsOfPosition,
167-
emitLeadingComments,
168-
emitTrailingComments,
169-
emitLeadingDetachedComments,
170-
emitTrailingDetachedComments
164+
emitNodeWithComments,
165+
emitBodyWithDetachedComments,
166+
emitTrailingCommentsOfPosition
171167
} = comments;
172168

173169
let context: TransformationContext;
174170
let getNodeEmitFlags: (node: Node) => NodeEmitFlags;
175171
let setNodeEmitFlags: (node: Node, flags: NodeEmitFlags) => void;
176172
let getSourceMapRange: (node: Node) => TextRange;
177173
let getTokenSourceMapRange: (node: Node, token: SyntaxKind) => TextRange;
178-
let getCommentRange: (node: Node) => TextRange;
179174
let isSubstitutionEnabled: (node: Node) => boolean;
180175
let isEmitNotificationEnabled: (node: Node) => boolean;
181176
let onSubstituteNode: (node: Node, isExpression: boolean) => Node;
@@ -240,7 +235,6 @@ const _super = (function (geti, seti) {
240235
setNodeEmitFlags = undefined;
241236
getSourceMapRange = undefined;
242237
getTokenSourceMapRange = undefined;
243-
getCommentRange = undefined;
244238
isSubstitutionEnabled = undefined;
245239
isEmitNotificationEnabled = undefined;
246240
onSubstituteNode = undefined;
@@ -262,7 +256,6 @@ const _super = (function (geti, seti) {
262256
setNodeEmitFlags = context.setNodeEmitFlags;
263257
getSourceMapRange = context.getSourceMapRange;
264258
getTokenSourceMapRange = context.getTokenSourceMapRange;
265-
getCommentRange = context.getCommentRange;
266259
isSubstitutionEnabled = context.isSubstitutionEnabled;
267260
isEmitNotificationEnabled = context.isEmitNotificationEnabled;
268261
onSubstituteNode = context.onSubstituteNode;
@@ -276,7 +269,7 @@ const _super = (function (geti, seti) {
276269
currentFileIdentifiers = node.identifiers;
277270
sourceMap.setSourceFile(node);
278271
comments.setSourceFile(node);
279-
emitNodeWithNotificationOption(node, emitWorker);
272+
emitNodeWithNotification(node, emitWorker);
280273
return node;
281274
}
282275

@@ -291,7 +284,7 @@ const _super = (function (geti, seti) {
291284
* Emits a node.
292285
*/
293286
function emit(node: Node) {
294-
emitNodeWithNotificationOption(node, emitWithoutNotificationOption);
287+
emitNodeWithNotification(node, emitWithComments);
295288
}
296289

297290
/**
@@ -315,51 +308,71 @@ const _super = (function (geti, seti) {
315308
}
316309

317310
/**
318-
* Emits a node without calling onEmitNode.
319-
* NOTE: Do not call this method directly.
311+
* Emits a node with comments.
312+
*
313+
* NOTE: Do not call this method directly. It is part of the emit pipeline
314+
* and should only be called indirectly from emit.
320315
*/
321-
function emitWithoutNotificationOption(node: Node) {
322-
emitNodeWithWorker(node, emitWorker);
316+
function emitWithComments(node: Node) {
317+
emitNodeWithComments(node, emitWithSourceMap);
318+
}
319+
320+
/**
321+
* Emits a node with source maps.
322+
*
323+
* NOTE: Do not call this method directly. It is part of the emit pipeline
324+
* and should only be called indirectly from emitWithComments.
325+
*/
326+
function emitWithSourceMap(node: Node) {
327+
emitNodeWithSourceMap(node, emitWorker);
323328
}
324329

325330
/**
326331
* Emits an expression node.
327332
*/
328333
function emitExpression(node: Expression) {
329-
emitNodeWithNotificationOption(node, emitExpressionWithoutNotificationOption);
334+
emitNodeWithNotification(node, emitExpressionWithComments);
330335
}
331336

332337
/**
333-
* Emits an expression without calling onEmitNode.
334-
* NOTE: Do not call this method directly.
338+
* Emits an expression with comments.
339+
*
340+
* NOTE: Do not call this method directly. It is part of the emitExpression pipeline
341+
* and should only be called indirectly from emitExpression.
335342
*/
336-
function emitExpressionWithoutNotificationOption(node: Expression) {
337-
emitNodeWithWorker(node, emitExpressionWorker);
343+
function emitExpressionWithComments(node: Expression) {
344+
emitNodeWithComments(node, emitExpressionWithSourceMap);
345+
}
346+
347+
/**
348+
* Emits an expression with source maps.
349+
*
350+
* NOTE: Do not call this method directly. It is part of the emitExpression pipeline
351+
* and should only be called indirectly from emitExpressionWithComments.
352+
*/
353+
function emitExpressionWithSourceMap(node: Expression) {
354+
emitNodeWithSourceMap(node, emitExpressionWorker);
338355
}
339356

340357
/**
341358
* Emits a node with emit notification if available.
342359
*/
343-
function emitNodeWithNotificationOption(node: Node, emit: (node: Node) => void) {
360+
function emitNodeWithNotification(node: Node, emitCallback: (node: Node) => void) {
344361
if (node) {
345362
if (isEmitNotificationEnabled(node)) {
346-
onEmitNode(node, emit);
363+
onEmitNode(node, emitCallback);
347364
}
348365
else {
349-
emit(node);
366+
emitCallback(node);
350367
}
351368
}
352369
}
353370

354-
function emitNodeWithWorker(node: Node, emitWorker: (node: Node) => void) {
371+
function emitNodeWithSourceMap(node: Node, emitCallback: (node: Node) => void) {
355372
if (node) {
356-
const leadingComments = getLeadingComments(/*range*/ node, /*contextNode*/ node, shouldSkipLeadingCommentsForNode, getCommentRange);
357-
const trailingComments = getTrailingComments(/*range*/ node, /*contextNode*/ node, shouldSkipTrailingCommentsForNode, getCommentRange);
358-
emitLeadingComments(/*range*/ node, leadingComments, /*contextNode*/ node, getCommentRange);
359373
emitStart(/*range*/ node, /*contextNode*/ node, shouldSkipLeadingSourceMapForNode, shouldSkipSourceMapForChildren, getSourceMapRange);
360-
emitWorker(node);
374+
emitCallback(node);
361375
emitEnd(/*range*/ node, /*contextNode*/ node, shouldSkipTrailingSourceMapForNode, shouldSkipSourceMapForChildren, getSourceMapRange);
362-
emitTrailingComments(node, trailingComments);
363376
}
364377
}
365378

@@ -1642,24 +1655,33 @@ const _super = (function (geti, seti) {
16421655
}
16431656

16441657
increaseIndent();
1645-
emitLeadingDetachedComments(body.statements, body, shouldSkipLeadingCommentsForNode);
16461658

1659+
emitBodyWithDetachedComments(body, body.statements,
1660+
shouldEmitBlockFunctionBodyOnSingleLine(parentNode, body)
1661+
? emitBlockFunctionBodyOnSingleLine
1662+
: emitBlockFunctionBodyWorker);
1663+
1664+
decreaseIndent();
1665+
writeToken(SyntaxKind.CloseBraceToken, body.statements.end, body);
1666+
}
1667+
1668+
function emitBlockFunctionBodyOnSingleLine(body: Block) {
1669+
emitBlockFunctionBodyWorker(body, /*emitBlockFunctionBodyOnSingleLine*/ true);
1670+
}
1671+
1672+
function emitBlockFunctionBodyWorker(body: Block, emitBlockFunctionBodyOnSingleLine?: boolean) {
16471673
// Emit all the prologue directives (like "use strict").
16481674
const statementOffset = emitPrologueDirectives(body.statements, /*startWithNewLine*/ true);
16491675
const helpersEmitted = emitHelpers(body);
16501676

1651-
if (statementOffset === 0 && !helpersEmitted && shouldEmitBlockFunctionBodyOnSingleLine(parentNode, body)) {
1677+
if (statementOffset === 0 && !helpersEmitted && emitBlockFunctionBodyOnSingleLine) {
16521678
decreaseIndent();
16531679
emitList(body, body.statements, ListFormat.SingleLineFunctionBodyStatements);
16541680
increaseIndent();
16551681
}
16561682
else {
16571683
emitList(body, body.statements, ListFormat.MultiLineFunctionBodyStatements, statementOffset);
16581684
}
1659-
1660-
emitTrailingDetachedComments(body.statements, body, shouldSkipTrailingCommentsForNode);
1661-
decreaseIndent();
1662-
writeToken(SyntaxKind.CloseBraceToken, body.statements.end, body);
16631685
}
16641686

16651687
function emitClassDeclaration(node: ClassDeclaration) {
@@ -2004,9 +2026,12 @@ const _super = (function (geti, seti) {
20042026
// }
20052027
// "comment1" is not considered to be leading comment for node.initializer
20062028
// but rather a trailing comment on the previous node.
2007-
if (!shouldSkipLeadingCommentsForNode(node.initializer)) {
2008-
emitLeadingComments(/*range*/ node.initializer, getTrailingComments(collapseRangeToStart(node.initializer)), /*contextNode*/ node.initializer, getCommentRange);
2029+
const initializer = node.initializer;
2030+
if (!shouldSkipLeadingCommentsForNode(initializer)) {
2031+
const commentRange = initializer.commentRange || initializer;
2032+
emitTrailingCommentsOfPosition(commentRange.pos);
20092033
}
2034+
20102035
emitExpression(node.initializer);
20112036
}
20122037

@@ -2034,8 +2059,10 @@ const _super = (function (geti, seti) {
20342059
function emitSourceFile(node: SourceFile) {
20352060
writeLine();
20362061
emitShebang();
2037-
emitLeadingDetachedComments(node);
2062+
emitBodyWithDetachedComments(node, node.statements, emitSourceFileWorker);
2063+
}
20382064

2065+
function emitSourceFileWorker(node: SourceFile) {
20392066
const statements = node.statements;
20402067
const statementOffset = emitPrologueDirectives(statements);
20412068
if (getNodeEmitFlags(node) & NodeEmitFlags.NoLexicalEnvironment) {
@@ -2049,8 +2076,6 @@ const _super = (function (geti, seti) {
20492076
emitList(node, statements, ListFormat.MultiLine, statementOffset);
20502077
tempFlags = savedTempFlags;
20512078
}
2052-
2053-
emitTrailingDetachedComments(node.statements);
20542079
}
20552080

20562081
// Transformation nodes
@@ -2326,7 +2351,8 @@ const _super = (function (geti, seti) {
23262351
}
23272352
else {
23282353
// Write the opening line terminator or leading whitespace.
2329-
let shouldEmitInterveningComments = true;
2354+
const mayEmitInterveningComments = (format & ListFormat.NoInterveningComments) === 0;
2355+
let shouldEmitInterveningComments = mayEmitInterveningComments;
23302356
if (shouldWriteLeadingLineTerminator(parentNode, children, format)) {
23312357
writeLine();
23322358
shouldEmitInterveningComments = false;
@@ -2369,10 +2395,11 @@ const _super = (function (geti, seti) {
23692395
}
23702396

23712397
if (shouldEmitInterveningComments) {
2372-
emitLeadingComments(/*node*/ child, getTrailingCommentsOfPosition(child.pos), /*contextNode*/ child, getCommentRange);
2398+
const commentRange = child.commentRange || child;
2399+
emitTrailingCommentsOfPosition(commentRange.pos);
23732400
}
23742401
else {
2375-
shouldEmitInterveningComments = true;
2402+
shouldEmitInterveningComments = mayEmitInterveningComments;
23762403
}
23772404

23782405
// Emit this child.
@@ -2876,6 +2903,7 @@ const _super = (function (geti, seti) {
28762903
// Other
28772904
PreferNewLine = 1 << 15, // Prefer adding a LineTerminator between synthesized nodes.
28782905
NoTrailingNewLine = 1 << 16, // Do not emit a trailing NewLine for a MultiLine list.
2906+
NoInterveningComments = 1 << 17,// Do not emit comments between each node
28792907

28802908
// Precomputed Formats
28812909
Modifiers = SingleLine | SpaceBetweenSiblings,
@@ -2890,7 +2918,7 @@ const _super = (function (geti, seti) {
28902918
ArrayLiteralExpressionElements = PreserveLines | CommaDelimited | SpaceBetweenSiblings | AllowTrailingComma | Indented | SquareBrackets,
28912919
CallExpressionArguments = CommaDelimited | SpaceBetweenSiblings | SingleLine | Parenthesis,
28922920
NewExpressionArguments = CommaDelimited | SpaceBetweenSiblings | SingleLine | Parenthesis | OptionalIfUndefined,
2893-
TemplateExpressionSpans = SingleLine,
2921+
TemplateExpressionSpans = SingleLine | NoInterveningComments,
28942922
SingleLineBlockStatements = SpaceBetweenBraces | SpaceBetweenSiblings | SingleLine,
28952923
MultiLineBlockStatements = Indented | MultiLine,
28962924
VariableDeclarationList = CommaDelimited | SpaceBetweenSiblings | SingleLine,
@@ -2902,8 +2930,8 @@ const _super = (function (geti, seti) {
29022930
EnumMembers = CommaDelimited | Indented | MultiLine,
29032931
CaseBlockClauses = Indented | MultiLine,
29042932
NamedImportsOrExportsElements = CommaDelimited | SpaceBetweenSiblings | AllowTrailingComma | SingleLine | SpaceBetweenBraces,
2905-
JsxElementChildren = SingleLine,
2906-
JsxElementAttributes = SingleLine | SpaceBetweenSiblings,
2933+
JsxElementChildren = SingleLine | NoInterveningComments,
2934+
JsxElementAttributes = SingleLine | SpaceBetweenSiblings | NoInterveningComments,
29072935
CaseOrDefaultClauseStatements = Indented | MultiLine | NoTrailingNewLine | OptionalIfEmpty,
29082936
HeritageClauseTypes = CommaDelimited | SpaceBetweenSiblings | SingleLine,
29092937
SourceFileStatements = MultiLine | NoTrailingNewLine,

src/compiler/scanner.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ namespace ts {
591591
* and the next token are returned.
592592
* If true, comments occurring between the given position and the next line break are returned.
593593
*/
594-
function getCommentRanges(text: string, pos: number, trailing: boolean, consumedCommentRanges?: Map<boolean>): CommentRange[] {
594+
function getCommentRanges(text: string, pos: number, trailing: boolean): CommentRange[] {
595595
let result: CommentRange[];
596596
let collecting = trailing || pos === 0;
597597
while (pos >= 0 && pos < text.length) {
@@ -643,15 +643,12 @@ namespace ts {
643643
}
644644
}
645645

646-
if (collecting && (!consumedCommentRanges || !(startPos in consumedCommentRanges))) {
646+
if (collecting) {
647647
if (!result) {
648648
result = [];
649649
}
650650

651651
result.push({ pos: startPos, end: pos, hasTrailingNewLine, kind });
652-
if (consumedCommentRanges) {
653-
consumedCommentRanges[startPos] = true;
654-
}
655652
}
656653

657654
continue;
@@ -673,12 +670,12 @@ namespace ts {
673670
return result;
674671
}
675672

676-
export function getLeadingCommentRanges(text: string, pos: number, consumedCommentRanges?: Map<boolean>): CommentRange[] {
677-
return getCommentRanges(text, pos, /*trailing*/ false, consumedCommentRanges);
673+
export function getLeadingCommentRanges(text: string, pos: number): CommentRange[] {
674+
return getCommentRanges(text, pos, /*trailing*/ false);
678675
}
679676

680-
export function getTrailingCommentRanges(text: string, pos: number, consumedCommentRanges?: Map<boolean>): CommentRange[] {
681-
return getCommentRanges(text, pos, /*trailing*/ true, consumedCommentRanges);
677+
export function getTrailingCommentRanges(text: string, pos: number): CommentRange[] {
678+
return getCommentRanges(text, pos, /*trailing*/ true);
682679
}
683680

684681
/** Optionally, get the shebang */

src/compiler/transformers/es6.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1526,6 +1526,7 @@ namespace ts {
15261526

15271527
const declarationList = createVariableDeclarationList(declarations, /*location*/ node);
15281528
setOriginalNode(declarationList, node);
1529+
setCommentRange(declarationList, node);
15291530

15301531
if (node.transformFlags & TransformFlags.ContainsBindingPattern
15311532
&& (isBindingPattern(node.declarations[0].name)

src/compiler/transformers/module/module.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ namespace ts {
1717
hoistVariableDeclaration,
1818
setNodeEmitFlags,
1919
getNodeEmitFlags,
20+
setSourceMapRange,
2021
} = context;
2122

2223
const compilerOptions = context.getCompilerOptions();
@@ -926,7 +927,13 @@ namespace ts {
926927
}
927928

928929
function createExportStatement(name: Identifier, value: Expression, location?: TextRange) {
929-
return startOnNewLine(createStatement(createExportAssignment(name, value), location));
930+
const statement = createStatement(createExportAssignment(name, value));
931+
statement.startsOnNewLine = true;
932+
if (location) {
933+
setSourceMapRange(statement, location);
934+
}
935+
936+
return statement;
930937
}
931938

932939
function createExportAssignment(name: Identifier, value: Expression) {

0 commit comments

Comments
 (0)