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

Improve the symbol importers API #7926

Closed
scabug opened this issue Oct 21, 2013 · 6 comments
Closed

Improve the symbol importers API #7926

scabug opened this issue Oct 21, 2013 · 6 comments

Comments

@scabug
Copy link

scabug commented Oct 21, 2013

Quoting Iulian:

I noticed a pretty serious problem regarding importers. As we stand now, the compiler is single-threaded, and the presentation compiler enforces this by checking no symbols are initialized outside the compiler thread.

An importer operates in between two compiler instances, and can force symbols on both sides. Therefore, in the presentation compiler there is no one correct thread on which to execute the call to `importSymbol`. There could be various hacks (like forcing a symbol and its owner chain on one thread, and then calling importSymbol on another thread), but they are error-prone and maybe don't even solve the issue.

We end up working around these limitations too much, and I think this needs to be fixed in the reflection library. Right now, we're forcing the owner chain of from.Symbol on "PC thread From" before calling importSymbol in "PC thread To". But that's not enough, and now we're going to also initialize companion classes/modules before calling importSymbol. There must be a better way...

Just for your own curiosity, this is how our code using symbol importer look like https://github.com/scala-ide/scala-search/pull/73/files

Which shows in code exactly what Iulian described. It feels too brittle to be right.

@scabug
Copy link
Author

scabug commented Oct 21, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7926?orig=1
Reporter: @dotta
Affected Versions: 2.11.0-M6

@scabug
Copy link
Author

scabug commented Oct 21, 2013

@retronym said:
Do you have a recommendation? Would you like us to offer the better version of fullyInitializeSymbol? Or call that automatically on your behalf somewhere?

@scabug
Copy link
Author

scabug commented Oct 21, 2013

@dotta said:
My feeling is that it should be transparent for the user. So, if there is any symbol that have to be initialized, it should be done automatically.

The other question is whether the current implementation of the symbol importers API could be improved to avoid the need of pre-initializing symbols before importing them in a different universe. I don't have enough insights to answer this, but I can imagine doing so may require foundamental changes in the compiler hearth, if possible at all. But the current design seems broken, and fragile. Can you really ensure that you will have initialized enough symbols?
If this is a known limitation, and there is no other way around it, then that's what it is, and we'll live with it.

@scabug
Copy link
Author

scabug commented Oct 21, 2013

@retronym said:
Maybe we can discuss a smaller example. Here's a starting point:

class Global(expectedThread: Thread) {
  def checkThread() { Thread.currentThread == expectedThread }
  def set(a: Any) = {checkThread(); ...  }
  lazy val get = {checkThread(); ...  }
}

def importer(g1: Global, g2: Global) = {
  g2.set(g1.get)
}

val g1 = new Global(thread1); val g2 = new Global(thread2)
onThread(thread2) {
  import(g1, g2)
}

Here, importer can never satisfy the checks for the correct thread. As I understand it, you workaround this by:

onThread(thread1) { g1.get }
onThread(thread2) { importer(g1, g2) }

In practice, there are a few more layers; You get Symbols out of Global, and some methods on the Symbol are lazy and need forcing.

Brainstorming solutions, we could:

  • find all APIs in the compiler that return a Symbol and decorate them for the presentation compiler with something that calls fullyInitialize
  • disable this thread check for the duration of the call to import.
  • rewrite Importers to execute calls against g1 and g2 via respectively provided ExecutorServices that do things on the correct thread.
  • ???

Let's chat about this tomorrow in person.

@scabug
Copy link
Author

scabug commented Dec 11, 2013

@xeno-by said:
I think this issue might be worth taking a second look before releasing 2.11.

@scabug
Copy link
Author

scabug commented Feb 6, 2014

@retronym said:
Closing as out of scope as a result of our discussions on this matter didn't find a workable answer,

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

2 participants