Skip to content

ce5c0b0727 (Jason Zaugg, 2 minutes ago) Encapsulate TypeHistory mutation and remove obviated optimization adaptInfos#73

Closed
retronym wants to merge 2 commits into2.13.xfrom
review/8463
Closed

ce5c0b0727 (Jason Zaugg, 2 minutes ago) Encapsulate TypeHistory mutation and remove obviated optimization adaptInfos#73
retronym wants to merge 2 commits into2.13.xfrom
review/8463

Conversation

@retronym
Copy link
Owner

6659f90 (Diego E. Alonso Blas, 5 hours ago)
TypeHistory: use mutability to avoid allocations.

Monitoring compilation with a flight recorder, we noticed that the compiler
allocates a lot of TypeHistory objects. TO reduce those allocations:

  • We extract a variable for the special TypeHistory instance that
    is used at the beginning, with "NoType" type.
  • We make all class fields mutable: when we are adding a new
    "root" (no previous history), we avoid creating a new "TypeHistory"
    and instead "rewrite" the existing one.

diesalbla and others added 2 commits October 14, 2019 03:28
Monitoring compilation with a flight recorder, we noticed that the
compiler allocates a lot of TypeHistory objects. TO reduce those
allocations:

- We extract a variable for the special `TypeHistory` instance that
  is used at the beginning, with "NoType" type.
- We make all class fields mutable: when we are adding a new
  "root" (no previous history), we avoid creating a new "TypeHistory"
  and instead "rewrite" the existing one.

// OPT: mutate the current TypeHistory rather than creating a new one. TypeHistory instances should not be shared.
final def reset(validFrom: Period, info: Type): TypeHistory = {
if (this ne noTypeHistory) TypeHistory(validFrom, info, null)

Choose a reason for hiding this comment

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

I think this has the branches switched... because NoTypeHistory is a constant to be shared, that is the one that should never be mutated.


// OPT: mutate the current TypeHistory rather than creating a new one. TypeHistory instances should not be shared.
final def reset(validFrom: Period, info: Type): TypeHistory = {
if (this ne noTypeHistory) TypeHistory(validFrom, info, null)
Copy link
Owner Author

Choose a reason for hiding this comment

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

You're right, I got this if the wrong way around.

@retronym retronym closed this Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants