Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Perf research: use scope analysis to circumvent type information for some cases #6217

bradzacher started this conversation in Technical Discussions
Discussion options

Type information is computed lazily by TS. What this means is that the more type information a rule accesses, the slower that rule is.

Right now all lint rules directly use TS's APIs to ask TS to compute types before operating on those types and asking questions of those types.

I wonder if we can improve the performance here by using the scope analysis information that's already calculated to get some basic types from the AST.
My hypothesis is that there are a lot of questions we're asking that could be answered solely from the AST.

For example:

const x = 1;

const result = x + 1;

For restrict-plus-operands the rule will inspect both sides of the binary expression, ask for types, and then run some predicates over the types to determine if the operation is safe or not.
In this example, however, we shouldn't ever need to ask TS anything - we could solve this entirely using the AST + scope analysis.

For example instead of doing (pseudocode):

BinaryExpression(node) {
  const left = ts.getType(node.left);
  const right = ts.getType(node.right);
  if (isString(left) && isString(right)) { return safe; }
  if (isNumber(left) && isNumber(right)) { return safe; }
  // ...
}

we could potentially circumvent some work by doing something like:

BinaryExpression(node) {
  const left = getTypeFast(node.left);
  const right = getTypeFast(node.right);
  if (isString(left) && isString(right)) { return safe; }
  if (isNumber(left) && isNumber(right)) { return safe; }
}

function getTypeFast(node) {
  const scopeAnalysisType = getScopeAnalysisType(node);
  if (scopeAnalysisType) {
    return scopeAnalysisType;
  }
  return ts.getType(node);
}

function getScopeAnalysisType(node) {
  if (node.type === 'Literal') { return typeof node.value }
  if (node.type === 'Identifier') {
    const variable = scopeAnalysis.get(node.name);
    return getScopeAnalysisType(variable.declaration);
  }
  if (
    node.type === 'VariableDeclaration' &&
    node.kind === 'const'
  ) {
    return getScopeAnalysisType(node.init);
  }
  if (
    node.type === 'Parameter' &&
    node.typeAnnotation
  ) {
    return typeOf(node.typeAnnotation);
  }
  return null;
}

It's a lot more code, for sure, but it would save us from having to dip into the types for a number of cases.

Some of the more complex cases (like looking for promises, where we need to deeply inspect the object properties) could be a lot faster if we can circumvent ever asking for the type because we know it's definitely a number.

It would probably be a bit awkward to use any utilities that are built here, but it could work well in conjunction with #6013 if we had our own type API surface to automate some of the short-circuiting for people.

You must be logged in to vote

Replies: 1 comment · 1 reply

Comment options

I wonder if we can improve the performance here by using the scope analysis information that's already calculated to get some basic types from the AST.

Hi, @bradzacher What do you think about providing fast predicates type checking utilities?
The "some cases" mentioned seem to be mostly about checking if it's a certain type or not. (especially any or whether it's nullable).

export function isAny(node: TSESTree.Node, service: ParserServices): boolean {
   const scopeAnalysisType = getScopeAnalysisType(node);
  if( [
    SCOPE_ANALYSIS_TYPE.Literal, 
    SCOPE_ANALYSIS_TYPE.FunctionExpression,
    SCOPE_ANALYSIS_TYPE.ArrayExpression,
    //...
    ].includes(scopeAnalysisType)
  ) {
     return false;
  }
   const type = services.getTypeAtLocation(node);
   return isTypeFlagSet(type, ts.TypeFlags.Any);
}

export function isNullable(node: TSESTree.Node): boolean {
     if( [
    SCOPE_ANALYSIS_TYPE.FunctionExpression,
    SCOPE_ANALYSIS_TYPE.ArrayExpression,
    //...
    ].includes(scopeAnalysisType)
  ) {
     return false;
  }
   const type = ts.getType(node);
   return isTypeFlagSet(type, ts.TypeFlags.Undefined | ts.TypeFlags.Null);
   //...
}
  • client code
// some-rule.ts

  if (isAny(node)) {
     context.report({ ... });
  } else {
       const type = services.getTypeAtLocation(node);
       // ...
  }

In this way,

  • While developing rule, we can use utilities without having to consider whether it's the case needs to be optimized.
  • For some rules, we may not want to know the detail types. (Promise, Array ...)
    • For example, some rules need to know whether a call expression returns Promise or not, but don't need what type Promise will resolve. In this case, there are more optimizable opportunities: skip ts type checking for function which has "async" keyword.
export function isPromise(node: TSESTree.Node): boolean {
   // 
   
   const functionDef = /* .. */;
   if (functionDef.async) return true;
   
   const type = ts.getType(node);
   //...
}
You must be logged in to vote
1 reply
@bradzacher
Comment options

bradzacher Feb 18, 2024
Maintainer Author

@JoshuaKGoldberg and I were chatting about this the other day.

Theres probably some scope to do something like ts-simple-type where we "hide" the TS type api via a custom representation so that we can mix-and-match and optimise things behind the scene.

For example we could do something like return { type: "number-literal", value: 1 } for const x = 1;. That would allow us to build more generic APIs instead of limiting us to hyper-targeted ones like "isAny".

Theres a cost to this, ofc, and it would be a decent chunk of work to build.
The upside here is that we could also abstract away some of the complicated bits of working with types. Eg we could hide away things like "get contextual type" or generic resolution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issues regarding performance
2 participants
Converted from issue

This discussion was converted from issue #6216 on December 15, 2022 02:43.

Morty Proxy This is a proxified and sanitized view of the page, visit original site.