Skip to content

do not apply subtype reduction if type set contains enum literals fro…#11368

Merged
vladima merged 4 commits into
masterfrom
vladima/do-not-reduce-enum-literals
Oct 4, 2016
Merged

do not apply subtype reduction if type set contains enum literals fro…#11368
vladima merged 4 commits into
masterfrom
vladima/do-not-reduce-enum-literals

Conversation

@vladima
Copy link
Copy Markdown
Contributor

@vladima vladima commented Oct 4, 2016

fix proposal for one of issues outlined in #10018.

repro case can boil down to:

// large enum
enum E {
   E1, E2, ... E1000
}
function run(n) {
    switch (n) {
        case 0: return [E.E1, E.E2];
        case 1: return [E.E2, E.E3];
        ...
    }
}

We ended up with union types made of enum literals and spent most of the time doing subtype reduction which will not make any changes.

Proposal: make a pass over type set in question and check if all types are enum literal types that originate from the same enum. If check succeeded - skip subtype reduction part

Compilation time for code in test case:

  • 1.8 - 0.8 s
  • 2.0 - ~40 s
  • nightly - ~43 s
  • proposed fix - 1.5s

// CC: @ahejlsberg

Comment thread src/compiler/checker.ts Outdated
let sameEnumMembers = true;
for (let i = 1; i < types.length; i++) {
const other = types[i];
if (!(other.flags & TypeFlags.EnumLiteral) || (getParentOfSymbol(first.symbol) !== getParentOfSymbol(other.symbol))) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you can propably cache this getParentOfSymbol(first.symbol)

Comment thread src/compiler/checker.ts
if (sameEnumMembers) {
return;
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe extract this logic into a separate isSetOfLiteralsFromSameEnum function and call that here. Otherwise, looks good to me.

@vladima vladima merged commit ebb17e8 into master Oct 4, 2016
@vladima vladima deleted the vladima/do-not-reduce-enum-literals branch October 4, 2016 23:32
@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Oct 4, 2016

@vladima can we port this to release-2.0.5

vladima added a commit that referenced this pull request Oct 5, 2016
#11368)

* do not apply subtype reduction if type set contains enum literals from the same enum

* do not re-read symbol for the first enum

* addressed PR feedback
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants