Skip to content

Use invokedynamic to statically cache materialized class tags (initially disabled by default)#7862

Closed
retronym wants to merge 2 commits intoscala:2.13.xfrom
retronym:topic/indy-classtag
Closed

Use invokedynamic to statically cache materialized class tags (initially disabled by default)#7862
retronym wants to merge 2 commits intoscala:2.13.xfrom
retronym:topic/indy-classtag

Conversation

@retronym
Copy link
Member

During compiler profiling, I've found cases where the repeated allocation
of s.r.ClassTags (a wrapper over j.l.Class) contributed to allocation
pressure.

This commit introduces a static cache at the materialization site using
invokedynamic. This is analagous to the way we cache Symbol literals.

@scala-jenkins scala-jenkins added this to the 2.13.1 milestone Mar 13, 2019
@retronym retronym modified the milestones: 2.13.1, 2.13.0-RC1 Mar 13, 2019
@retronym
Copy link
Member Author

Marking for (late) consideration for 2.13.0 because binary compatibility constraints would not allow us to add the bootstrap method in 2.13.x

@retronym retronym added the performance the need for speed. usually compiler performance, sometimes runtime performance. label Mar 13, 2019
@lrytz
Copy link
Member

lrytz commented Mar 13, 2019

Could ClassTag be a value class / opaque type?

@retronym
Copy link
Member Author

That sounds like a good use case for value classes, but it would take a bit of work to untangle the current design from ClassManifest

@NthPortal
Copy link
Contributor

Could ClassTag be a value class / opaque type?

One issue is that ClassTag also has a newArray method which j.l.Class does not

@retronym
Copy link
Member Author

An alternative approach to invokeydynamcic-based, per-call-site caching, would be to use a global runtime cache. The JDK has a suitable data structure for this: ClassValue, and that is lock-free, and avoids classloader leaks.

#7879

@lrytz
Copy link
Member

lrytz commented Mar 15, 2019

it would take a bit of work to untangle the current design from ClassManifest

Yes :-) But maybe it's time to do that, instead of adding more complexity (even if hidden behind the scenes) to ClassTag?

One issue is that ClassTag also has a newArray method which j.l.Class does not

I don't see why that wouldn't work though?

ClassValue

Right, that seems like a good fit.

@retronym retronym force-pushed the topic/indy-classtag branch from c33cff5 to c04b7b9 Compare March 15, 2019 12:47
@retronym
Copy link
Member Author

Pulling this thread a little further, I'm trying to unify symbol literal, structural call site, and class tag caching behind a single bootstrap method.

@retronym retronym force-pushed the topic/indy-classtag branch 2 times, most recently from c97a79b to e118f59 Compare March 18, 2019 00:50
During compiler profiling, I've found cases where the repeated allocation
of `s.r.ClassTags` (a wrapper over j.l.Class) contributed to allocation
pressure.

This commit introduces a static cache at the materialization site
using invokedynamic. This is analagous to the way we cache Symbol
literals.
@retronym retronym force-pushed the topic/indy-classtag branch from e118f59 to 6f7ec45 Compare March 18, 2019 05:16
@adriaanm adriaanm added the prio:blocker release blocker (used only by core team, only near release time) label Mar 19, 2019
@retronym retronym added the WIP label Mar 20, 2019
@adriaanm adriaanm modified the milestones: 2.13.0-RC1, 2.13.1 Mar 21, 2019
@retronym retronym closed this May 30, 2019
@SethTisue SethTisue removed this from the 2.13.1 milestone May 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance the need for speed. usually compiler performance, sometimes runtime performance. prio:blocker release blocker (used only by core team, only near release time)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants