Pre compute suggestion db during build time #5698

4e6 · 2023-02-20T08:37:49Z

Pull Request Description

Cache suggestions during the buildEngineDistribution command, and read them from the disk when the library is loaded. Initial graph coloring takes ~20 seconds vs ~25 seconds on the develop branch.

peek-develop-branch.webm
peek-suggestions-branch.webm

Important Notes

Checklist

Please include the following checklist in your PR:

The documentation has been updated if necessary.
All code conforms to the
Scala,
Java,
and
Rust
style guides.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed: Enso GUI was tested when built using BOTH
  ./run ide build and ./run ide watch.

JaroslavTulach

Thanks Dmitry, I think I know what to do now.

engine/runtime/src/main/scala/org/enso/compiler/Compiler.scala

engine/runtime/src/main/java/org/enso/interpreter/runtime/Module.java

project/DistributionPackage.scala

engine/runtime/src/main/scala/org/enso/compiler/SuggestionsSerializationManager.scala

JaroslavTulach · 2023-02-21T07:56:43Z

engine/language-server/src/main/scala/org/enso/languageserver/search/SuggestionsHandler.scala

@@ -493,6 +465,13 @@ final class SuggestionsHandler(
      updates.foreach(sessionRouter ! _)
      context.become(initialized(name, graph, clients, state))

+    case libraryLoaded: Api.LibraryLoaded =>
+      logger.debug(


@4e6's idea is to deliver the cached suggestion.json here. However I haven't hit a breakpoint placed here yet. I am not sure who's supposed to deliver this message? Probably we are missing a call to NotificationHandler.addListener that would register SuggestionHandler. Probably the listener shall be attached when case InitializedEvent.TruffleContextInitialized is delivered - by where can I find the EnsoContext at that moment?

One has to subscribe to Akka event bus to get the notification - f480e03

But now there is another problem: The SuggestionSerializationManager is in runtime, while this code is in language-server - how shall these two talk to each other? I believe this "library added" notification is send from following stacktrace:

at org.enso.interpreter.instrument.Endpoint.sendToClient(Handler.scala:48) at org.enso.interpreter.instrument.NotificationHandler$InteractiveMode.sendMessage(NotificationHandler.scala:110) at org.enso.interpreter.instrument.NotificationHandler$InteractiveMode.addedLibrary(NotificationHandler.scala:123) at org.enso.interpreter.instrument.NotificationHandler$Forwarder.$anonfun$addedLibrary$1(NotificationHandler.scala:80) at org.enso.interpreter.instrument.NotificationHandler$Forwarder.$anonfun$addedLibrary$1$adapted(NotificationHandler.scala:79) at scala.collection.immutable.List.foreach(List.scala:333) at org.enso.interpreter.instrument.NotificationHandler$Forwarder.addedLibrary(NotificationHandler.scala:79) at org.enso.compiler.PackageRepository$Default.registerPackageInternal(PackageRepository.scala:326)

I believe the next step is to modify code in PackageRepository to check whether a suggestion cache exists for the library. If it does, read it and send the content of the suggestions via the Api.LibraryLoaded messages to the "language-server" project that can distribute it further. The following code shall be modified to send also librarySuggestions:

if (isLibrary) { val root = Path.of(pkg.root.toString) notificationHandler.addedLibrary(libraryName, libraryVersion, root) }

project/DistributionPackage.scala

…mpute-suggestion-db-during-build-time

Added a last-modified cache for stdlib files so that we don't generate index every single time.

…' of enso:enso-org/enso into wip/db/5068-pre-compute-suggestion-db-during-build-time

engine/runtime/src/test/java/org/enso/compiler/test/context/JacksonTest.java

engine/runtime/src/main/scala/org/enso/compiler/context/SuggestionDiff.scala

…ing-build-time

engine/runtime/src/main/java/org/enso/compiler/Cache.java

JaroslavTulach · 2023-03-06T03:57:44Z

engine/runtime/src/main/java/org/enso/compiler/SuggestionsCache.java

+
+  @Override
+  protected byte[] metadata(String sourceDigest, String blobDigest, CachedSuggestions entry) {
+    var mapper = new ObjectMapper();


Shouldn't the mapper be a field in the cache? Unless I am mistaken the ObjectMapper instance maintains mapping from Class to "functionality". If we create new instance for every library, then its internal mapping must be initialized every time.

It seems that since version 2, the ObjectMapper creation is cheap https://stackoverflow.com/questions/18611565/how-do-i-correctly-reuse-jackson-objectmapper#comment27463038_18615839

That's not what I experienced. This causes major slowdowns, which is why I also changed it in #5791

JaroslavTulach · 2023-03-06T04:02:43Z

engine/runtime/src/main/java/org/enso/compiler/SuggestionsCache.java

+    }
+  }
+
+  // CachedSuggestions class is not a record because of a Frgaal bug leading to invalid compilation error.


What is the error? Maybe updating to new version of frgaal would fix the problem. If I modify:

enso$ git diff diff --git engine/runtime/src/main/java/org/enso/compiler/SuggestionsCache.java engine/runtime/src/main/java/org/enso/compiler/SuggestionsCache.java index f5d65b8e0..220c01e9c 100644 --- engine/runtime/src/main/java/org/enso/compiler/SuggestionsCache.java +++ engine/runtime/src/main/java/org/enso/compiler/SuggestionsCache.java @@ -125,16 +125,7 @@ public final class SuggestionsCache } // CachedSuggestions class is not a record because of a Frgaal bug leading to invalid compilation error. - public final static class CachedSuggestions { - - private final LibraryName libraryName; - private final Suggestions suggestions; - - public CachedSuggestions(LibraryName libraryName, Suggestions suggestions) { - this.libraryName = libraryName; - this.suggestions = suggestions; - } - + public record CachedSuggestions(LibraryName libraryName, Suggestions suggestions) { public LibraryName getLibraryName() { return libraryName; }

I am seeing:

sbt:enso> buildEngineDistribution [info] compiling 45 Scala sources and 191 Java sources to /enso/engine/runtime/target/scala-2.13/classes ... [error] /enso/engine/runtime/src/main/java/org/enso/compiler/SuggestionsCache.java:128:17: not found: type java [error] public record CachedSuggestions(LibraryName libraryName, Suggestions suggestions) { [error] ^ [error] one error found

That's a wild error. Reported to the frgaal project.

Yes, this is what I was seeing as well and the reason why in other cases I don't use the record. Thanks for reporting it with frgaal

JaroslavTulach · 2023-03-06T04:15:52Z

engine/runtime/src/main/scala/org/enso/interpreter/instrument/NotificationHandler.scala

+          name      = libraryName.name,
+          version   = libraryVersion.toString,
+          location  = location.toFile
+        )
      )


I thought that the Api.LibraryLoaded should be enhanced with optional list of suggestions. However it looks like the proposal is to send a message to self and deserialize the suggestions later. Assuming the deserialization is instant - is there any benefit of sending the suggestions in a separate message?

…ing-build-time

JaroslavTulach · 2023-03-08T09:23:16Z

engine/language-server/src/test/scala/org/enso/languageserver/search/Suggestions.scala

@@ -1,6 +1,7 @@
 package org.enso.languageserver.search

 import org.enso.docs.generator.DocsGenerator
+import org.enso.docs.sections.DocSectionsBuilder


Probably unnecessary import.

…ing-build-time

radeusgd

The change in micro-distribution looks OK.

hubertp

Minor nits but overall looks ok

hubertp · 2023-03-08T10:04:33Z

engine/language-server/src/main/scala/org/enso/languageserver/search/SuggestionsHandler.scala

+        libraryLoaded.namespace,
+        libraryLoaded.name
+      )
+      System.err.println("Loaded library: " + libraryLoaded.name)


This looks spurious

hubertp · 2023-03-08T10:05:43Z

engine/language-server/src/main/scala/org/enso/languageserver/search/SuggestionsHandler.scala

+    } yield {
+      val treeUpdates = treeResults.flatMap {
+        case QueryResult(ids, Api.SuggestionUpdate(suggestion, action)) =>
+          val verb = action.getClass.getSimpleName


can be moved to the condition with the log using it?

hubertp · 2023-03-08T10:25:31Z

engine/runtime/src/main/java/org/enso/compiler/SuggestionsCache.java

+
+  @Override
+  protected byte[] metadata(String sourceDigest, String blobDigest, CachedSuggestions entry) {
+    var mapper = new ObjectMapper();


That's not what I experienced. This causes major slowdowns, which is why I also changed it in #5791

hubertp · 2023-03-08T10:25:31Z

engine/runtime/src/main/java/org/enso/compiler/SuggestionsCache.java

+
+  @Override
+  protected byte[] metadata(String sourceDigest, String blobDigest, CachedSuggestions entry) {
+    var mapper = new ObjectMapper();


That's not what I experienced. This causes major slowdowns, which is why I also changed it in #5791

hubertp · 2023-03-08T10:26:44Z

engine/runtime/src/main/java/org/enso/compiler/SuggestionsCache.java

+    }
+  }
+
+  // CachedSuggestions class is not a record because of a Frgaal bug leading to invalid compilation error.


Yes, this is what I was seeing as well and the reason why in other cases I don't use the record. Thanks for reporting it with frgaal

hubertp · 2023-03-08T10:28:21Z

engine/runtime/src/main/scala/org/enso/compiler/SerializationManager.scala

+        }
+
+      try {
+        val suggestions = new util.ArrayList[Suggestion]()


shouldn't this rather be refactored to a separate method?

hubertp · 2023-03-09T12:54:37Z

@4e6 I think documentation needs to be updated as well?

4e6 added 5 commits February 20, 2023 11:35

build: indexStdLib

b95a302

feat: serialization manager

6bb4d43

feat: update EnsureCompiledJob

9c84027

feat: simplify suggestions init

b84603e

misc: cleanup

1ee00d3

JaroslavTulach reviewed Feb 20, 2023

View reviewed changes

JaroslavTulach self-assigned this Feb 20, 2023

JaroslavTulach added 2 commits February 20, 2023 15:39

Serialize provided suggestions to each standard library

e2ac361

Demo of iteratively adding constructing Tree

d62ec42

JaroslavTulach reviewed Feb 21, 2023

View reviewed changes

JaroslavTulach mentioned this pull request Feb 21, 2023

Investigate transfer of Suggestion DB to IDE as binary not as JSON #5570

Open

3 tasks

hubertp reviewed Feb 21, 2023

View reviewed changes

project/DistributionPackage.scala Outdated Show resolved Hide resolved

unfurl-links bot mentioned this pull request Feb 22, 2023

NPE in io.circe.Printer$PrintingFolder.onString from Client.processMessage( #5683

Closed

2 tasks

JaroslavTulach and others added 5 commits February 22, 2023 05:21

Merge remote-tracking branch 'origin/develop' into wip/db/5068-pre-co…

560a107

…mpute-suggestion-db-during-build-time

Only regenerate index when necessary

b68da75

Added a last-modified cache for stdlib files so that we don't generate index every single time.

Merge branch 'wip/db/5068-pre-compute-suggestion-db-during-build-time…

8f7314e

…' of enso:enso-org/enso into wip/db/5068-pre-compute-suggestion-db-during-build-time

Using Jackson to serialize and deserialize Suggestion.Module

2fbdf75

Can serialize Suggestion[]

f413b62

JaroslavTulach reviewed Feb 23, 2023

View reviewed changes

engine/runtime/src/test/java/org/enso/compiler/test/context/JacksonTest.java Show resolved Hide resolved

JaroslavTulach added 2 commits February 23, 2023 20:20

Can serialize record with List<Suggestion>

958cc78

Serialize and deserialize the Suggestion list as a case class

1ff4c25

JaroslavTulach reviewed Feb 24, 2023

View reviewed changes

engine/runtime/src/main/scala/org/enso/compiler/context/SuggestionDiff.scala Outdated Show resolved Hide resolved

JaroslavTulach added 3 commits February 24, 2023 08:41

Can load back the Tree.Node[Suggestion] from a cache

ce821b1

Less debug messages

5bff1ab

subscribe to Api.LibraryLoaded

f480e03

enso-bot bot mentioned this pull request Feb 25, 2023

Dropdown for Table.join right is wrong #5055

Closed

4e6 added 3 commits March 2, 2023 19:37

Merge branch 'develop' into wip/db/5068-pre-compute-suggestion-db-dur…

335305a

…ing-build-time

feat: suggestions cache

b4a98a4

feat: suggestions deserialization

6638c9d

4e6 force-pushed the wip/db/5068-pre-compute-suggestion-db-during-build-time branch from 2215f5a to 6638c9d Compare March 5, 2023 22:45

JaroslavTulach reviewed Mar 6, 2023

View reviewed changes

4e6 added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 6, 2023

4e6 added 2 commits March 6, 2023 16:07

feat: generate docs

283b6f5

misc: format

9081707

4e6 marked this pull request as ready for review March 6, 2023 16:35

4e6 added 11 commits March 6, 2023 20:04

fix: suggestions loading

0e769c1

misc: run deserialization in background

6a96e38

test: fix language-server

77c5a2b

test: fix runtime

009b595

Merge branch 'develop' into wip/db/5068-pre-compute-suggestion-db-dur…

31773a6

…ing-build-time

test: fix RuntimeServerTest

a45898a

test: fix RuntimeErrorsTest

5314e10

test: fix RuntimeInstrumentTest

1447ed0

test: fix RuntimeSuggestionUpdatesTest

20c2e40

test: fix RuntimeStdlibTest

89739c0

test: fix RuntimeComponentsTest

36862c6

4e6 requested review from jdunkerley and radeusgd as code owners March 6, 2023 22:05

misc: cleanup

3468399

enso-bot bot mentioned this pull request Mar 7, 2023

Throwing an unhandled exception in Polyglot Java results in a null in the UI and no caught error message #5260

Closed

JaroslavTulach approved these changes Mar 8, 2023

View reviewed changes

4e6 added the CI: Ready to merge This PR is eligible for automatic merge label Mar 8, 2023

Merge branch 'develop' into wip/db/5068-pre-compute-suggestion-db-dur…

952f0f6

…ing-build-time

radeusgd approved these changes Mar 8, 2023

View reviewed changes

hubertp reviewed Mar 8, 2023

View reviewed changes

misc: review comments

4742f5f

mergify bot merged commit 9397a6e into develop Mar 8, 2023

mergify bot deleted the wip/db/5068-pre-compute-suggestion-db-during-build-time branch March 8, 2023 12:37

hubertp mentioned this pull request Mar 17, 2023

buildStdLib*** tasks do not re-generate the corresponding stdlib index #5999

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pre compute suggestion db during build time #5698

Pre compute suggestion db during build time #5698

4e6 commented Feb 20, 2023 •

edited

Loading

JaroslavTulach left a comment

JaroslavTulach Feb 21, 2023 •

edited

Loading

JaroslavTulach Feb 24, 2023 •

edited

Loading

JaroslavTulach Mar 6, 2023

4e6 Mar 6, 2023

hubertp Mar 8, 2023

hubertp Mar 8, 2023

JaroslavTulach Mar 6, 2023 •

edited

Loading

hubertp Mar 8, 2023

JaroslavTulach Mar 6, 2023

JaroslavTulach Mar 8, 2023

radeusgd left a comment

hubertp left a comment

hubertp Mar 8, 2023

hubertp Mar 8, 2023

hubertp Mar 8, 2023

hubertp Mar 8, 2023

hubertp Mar 8, 2023

hubertp Mar 8, 2023

hubertp commented Mar 9, 2023

Pre compute suggestion db during build time #5698

Pre compute suggestion db during build time #5698

Conversation

4e6 commented Feb 20, 2023 • edited Loading

Pull Request Description

Important Notes

Checklist

JaroslavTulach left a comment

Choose a reason for hiding this comment

JaroslavTulach Feb 21, 2023 • edited Loading

Choose a reason for hiding this comment

JaroslavTulach Feb 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JaroslavTulach Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radeusgd left a comment

Choose a reason for hiding this comment

hubertp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hubertp commented Mar 9, 2023

4e6 commented Feb 20, 2023 •

edited

Loading

JaroslavTulach Feb 21, 2023 •

edited

Loading

JaroslavTulach Feb 24, 2023 •

edited

Loading

JaroslavTulach Mar 6, 2023 •

edited

Loading