Skip to content

Optimize union and intersection types#21573

Merged
ahejlsberg merged 2 commits into
masterfrom
optimizeUnionIntersection
Feb 2, 2018
Merged

Optimize union and intersection types#21573
ahejlsberg merged 2 commits into
masterfrom
optimizeUnionIntersection

Conversation

@ahejlsberg
Copy link
Copy Markdown
Member

This PR optimizes creation of union and intersection types. Previously we would use dynamically added properties to track attributes of the types in the union/intersection. Now we instead use a flags enum.

Also, we now initialize more common properties in the Symbol constructor which reduces polymorphism.

Performance of the check phase when compiling the compiler improves by 6-7% with these changes.

Comment thread src/compiler/checker.ts
if (includes & TypeIncludes.Union) {
// We are attempting to construct a type of the form X & (A | B) & Y. Transform this into a type of
// the form X & A & Y | X & B & Y and recursively reduce until no union type constituents remain.
const unionIndex = findIndex(typeSet, t => (t.flags & TypeFlags.Union) !== 0);
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.

Rather than calculating this all the time (which could get expensive for larger intersections), could we not reserve multiple bits of the flags for the UnionIndex (all the remaining free bits, even), set them with the index as types are added like before, and only perform this lookup if it's saturated (indicating that all we know when it's saturated is that the index is > or = the size of the saturated number, but then at least we can start searching from there)? Would prevent us from needing to do this lookup on intersections where a union appears within the first 2^20-1 or so members, and let us skip those first members if it's after them. This probably won't be noticable until you started defining a generic intersection with a few hundred plus elements where one of the last few elements is occasionally a union, but still.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I considered that, but compared to the amount of work we do to actually distribute over the union type it, and the relative rarity of this operation in the first place, it just seemed like extra complexity.

Copy link
Copy Markdown
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

I ran the rest of the performance tests. The change helps the compiler (both versions) and Angular. It doesn't help Monaco or TFS.

Interestingly, the emitter gets faster for the compiler and Angular. The change doesn't help Angular in the checker. I guess this is because Angular doesn't use unions all that much, and the emitter gain comes from reduced polymorphism.

@ahejlsberg ahejlsberg merged commit 79d2772 into master Feb 2, 2018
@ahejlsberg ahejlsberg deleted the optimizeUnionIntersection branch February 2, 2018 20:41
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 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