Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce allocations of AsSeenFromMap / FindMember / SubstSymMap #498

Open
retronym opened this issue May 8, 2018 · 4 comments
Open

Reduce allocations of AsSeenFromMap / FindMember / SubstSymMap #498

retronym opened this issue May 8, 2018 · 4 comments

Comments

@retronym
Copy link
Member

retronym commented May 8, 2018

image

It might be worth having a one-element cache in Global for each of these. We'd need to make them reusable (ie, the fields become vars and we add a reset method to assign new values).

The code in these can re-enter, at least through lazy type completers, so we either need to keep a stack of reusable ones, or simply just allocate for the re-entrant cases.

@retronym
Copy link
Member Author

retronym commented May 9, 2018

Here's what I've got in mind. scala/scala@9766d5d...retronym:faster/one-elem-cache

@retronym
Copy link
Member Author

I've taken a look at what would be needed to have the VM do this for us, with inlining and escape analysis.

One can use JITWatch:

object Test {
  val g = new scala.tools.nsc.Global(new scala.tools.nsc.Settings)
  g.settings.usejavacp.value = true
  import g._

  def main(args: Array[String])	{
    new Run()
    while (true) {
      test()
    }
  }

  def test() {
    typeOf[String].members
    typeOf[String].member(TermName("charAt"))
    typeOf[String].hasNonPrivateMember(TermName("charAt"))
  }
}
% qscalac sandbox/test.scala
% (qscala -J-XX:+UnlockDiagnosticVMOptions -J-XX:+TraceClassLoading -J-XX:+LogCompilation -J-XX:LogFile=/tmp/hotspot.log -J-XX:+PrintAssembly Test 2>&1) > /tmp/all.log

% cd /code; git clone jitwatch/jitwatch; cd jitwatch
% export MAVEN_OPTS="-Xmx4G"
% mvn clean compile test exec:java
  • Configure JitWatch with the location of the Scala sources and binaries:

image

  • Open Log /tmp/hotspot.log
  • Start
  • Navigate to scala.reflect.internal.Types$Type / findMember

image

  • 'Chain'

For escape analysis to kick in, we need FindMember.{<init>, apply, searchConcreteThenDeferred, walkBaseClasses, etc} all to inline, so that the FindMember instance doesn't escape outside the inlined unit of code.

With default VM options, we find this doesn't happen. Typically the reason in "already compiled into a big method".

image

This means that a helper method within FindMember had previously been chosen as the root of a JIT compilation task, and ended up with native code that exceeds a threshold in the JIT policy which prevents it from also being inlined into the call-tree that is the focus of the current JIT compilation task.

We can force inlining with a compiler control hint:

scala -J-XX:CompileCommand='inline scala.reflect.internal.tpe.FindMembers$FindMemberBase::*' ...

In which case we eventually can see escape analysis kick in:

image

(The -Allocs button in the JITWatch GUI shows all successfully eliminated allocations found in the compilation log.

Graal EE does a bit better at getting inlining some, but not all of these instances. Currently, HotSpot's profiling doesn't gather data about how much an allocation site contributes to the allocation rate of short lived objects. If it did, Graal could conceivably try harder to inline the allocation away.

hrhino added a commit to hrhino/scala that referenced this issue Nov 21, 2018
Even after 30+ iterations, I was unable to get the JIT to eliminate
these allocations.

Add in the single-element cache that could be used for scala/scala-dev#498.
hrhino added a commit to hrhino/scala that referenced this issue Nov 21, 2018
Even after 30+ iterations, I was unable to get the JIT to eliminate
these allocations.

Add in the single-element cache that could be used for scala/scala-dev#498.
hrhino added a commit to hrhino/scala that referenced this issue Nov 22, 2018
Even after 30+ iterations, I was unable to get the JIT to eliminate
these allocations.

Add in the single-element cache that could be used for scala/scala-dev#498.
hrhino added a commit to hrhino/scala that referenced this issue Dec 5, 2018
Even after 30+ iterations, I was unable to get the JIT to eliminate
these allocations.

Add in the single-element cache that could be used for scala/scala-dev#498.
@dwijnand
Copy link
Member

dwijnand commented Nov 4, 2020

I feel like I've seen PRs and 2.12/2.13 differences around this (from @diesalbla and @hrhino's handiwork?). Is this done / happy to close it now?

@dwijnand dwijnand added this to the Backlog milestone Nov 4, 2020
@diesalbla
Copy link

diesalbla commented Nov 5, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants