Optimize import/export resolution #5700

hubertp · 2023-02-20T10:22:39Z

Pull Request Description

This change adds serialization and deserialization of library bindings.
In order to be functional, one needs to first generate IR and
serialize bindings using --compiled <path-to-library> command. The bindings
will be stored under the library with .bindings suffix.
Bindings are being generated during buildEngineDistribution task, thus not
requiring any extra steps.

When resolving import/exports the compiler will first try to load
module's bindings from cache. If successful, it will not schedule its
imports/exports for immediate compilation, as we always did, but use the
bindings info to infer the dependent modules.

The current change does not make any optimizations when it comes to
compiling the modules, yet. It only delays the actual
compilation/loading IR from cache so that it can be done in bulk.
Further optimizations will come from this opportunity such as parallel
loading of caches or lazily inferring only the necessary modules.

Part of #5568 work.

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

Shared infrastructure for caching is something other work, including #5068, shall benefit from. Looks good. Few (sometimes unrelated) comments left.

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

JaroslavTulach · 2023-02-22T06:21:36Z

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

+
+}
+
+object Cache {


Having a shared access to caches is a great move forward. I would, however, prefer if the new code was written in Java rather than Scala. If this was a new code (as I realized later)...

This was a big one but I did it just for you :)

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

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

JaroslavTulach · 2023-02-22T06:51:22Z

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

@@ -146,7 +259,7 @@ class SerializationManager(compiler: Compiler) {
      abort(module)
      None
    } else {
-      while (isSerializing(module)) {
+      while (isSerializingModule(module)) {
        Thread.sleep(100)


wait(100) and a notifyAll() when serialization is over would be a nicer that unconditional sleep.

engine/runtime/src/main/scala/org/enso/compiler/data/BindingsMap.scala

engine/runtime/src/main/scala/org/enso/compiler/phase/ImportResolver.scala

This change adds serialization and deserialization of library bindings. In order to be functional, one can needs to first generate IR and serialize bindings `--compiled <path-to-library>` command. The bindings will be stored under the library with `.bindings` suffix. When resolving import/exports the compiler will first try to load module's bindings from cache. If successful, it will not schedule its imports/exports for immediate compilation, as we always did, but use the bindings info to infer the dependent modules. The current change does not make any optimizations when it comes to compiling the modules, yet. It only delays the actual compilation/loading IR from cache so that it can be done in bulk. Further optimizations will come from this opportunity such as parallel loading of caches or lazilly inferring only the necessary modules. Other important things to note: - deserialization has to take into account dependencies of modules, otherwise one may not be able to fully restore abstract module references - bindings map must have a stable serialization uid in order to avoid spurious and costly failed cache loading attempts

Race-condition could happen (as discovered by the failing test) if we were serializing sources read from the module rather than the cache entry. Had to modify the interface for consistency.

Seems like my code managed to trigger a case when comparing an `AtomConstructor` wrapped in a `Warning` with another `AtomConstructor`. Somehow we are already past `equalsWithWarnings` and enter the fallback specialization. Not yet sure how to reproduce this in a single test but my weird failing test in `Integrations_Tests` now succeeds.

Originally designed by Dmitry for generating suggestions during build time, it also works for generating library caches.

Compiler needs to run ExportResolution's sorting of graph nodes to ensure the correct order of resolved modules. Otherwise some exported symbols are resolved before being added to module's scope and lead to runtime No_Such_Method errors that are hart to pin down. Also added an optional way to parse modules in parallel which is turned off by default. This will allow for easier experimentation on how much impact that has.

Slowly moving the new code to Java, as per previous discussions. Added some duplicate API for Java to avoid horrible wrapping with converters that will quickly get out of hand. Because of a bug in Frgaal, had to use a simple record-like class rather than record itself.

JaroslavTulach · 2023-03-01T09:45:48Z

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

      if (
        module
-          .wasLoadedFromCache() && modules.exists(!_.wasLoadedFromCache())
+          .wasLoadedFromCache() && modules
+          .exists(m => !m.wasLoadedFromCache() && !m.isSynthetic)


This !m.isSynthetic check probably addresses #5608

hubertp force-pushed the wip/hubert/optimizing-export-resolution-5568 branch from 4313473 to afdcd0a Compare February 21, 2023 14:52

JaroslavTulach reviewed Feb 22, 2023

View reviewed changes

hubertp force-pushed the wip/hubert/optimizing-export-resolution-5568 branch 2 times, most recently from 4e02c06 to d280271 Compare February 24, 2023 11:21

hubertp changed the title ~~wip: Optimize import/export resolution~~ Optimize import/export resolution Feb 24, 2023

hubertp marked this pull request as ready for review February 24, 2023 14:12

hubertp requested a review from 4e6 as a code owner February 24, 2023 14:12

hubertp force-pushed the wip/hubert/optimizing-export-resolution-5568 branch from 1068f9e to 8b76359 Compare February 24, 2023 14:12

hubertp mentioned this pull request Feb 24, 2023

Downgrade hashing from SHA-3 to SHA-1 and eliminate unnecessary unnecessary digest #5763

Closed

JaroslavTulach approved these changes Feb 28, 2023

View reviewed changes

hubertp force-pushed the wip/hubert/optimizing-export-resolution-5568 branch from 8b76359 to b38db88 Compare February 28, 2023 11:56

hubertp added 17 commits February 28, 2023 14:57

DRY

5a35a54

Adapt test to changes

538a772

Fix invalidation of caches

628b5df

Avoid race-condition

3b7c217

Race-condition could happen (as discovered by the failing test) if we were serializing sources read from the module rather than the cache entry. Had to modify the interface for consistency.

Steal solution for generating library caches

8fc6d0c

Originally designed by Dmitry for generating suggestions during build time, it also works for generating library caches.

We don't have enso.exe on Windows

5cb6d75

update changelog

a4efa04

update docs

91e0273

Windows friendly executable

59bcb56

Another attempt to fix Windows build

ead5221

Drop Java suffix and Scala equivalents

d9ade44

Fix test

3ce6143

fmt

bde7304

hubertp force-pushed the wip/hubert/optimizing-export-resolution-5568 branch from 501f16d to bde7304 Compare February 28, 2023 13:57

nits

5546226

More test fixes

7e81fa5

hubertp added the CI: Ready to merge This PR is eligible for automatic merge label Mar 1, 2023

mergify bot merged commit 941512e into develop Mar 1, 2023

mergify bot deleted the wip/hubert/optimizing-export-resolution-5568 branch March 1, 2023 08:53

JaroslavTulach reviewed Mar 1, 2023

View reviewed changes

JaroslavTulach mentioned this pull request Mar 1, 2023

Synthetic modules are not being serialized causing constant cache invalidation #5608

Closed

jdunkerley mentioned this pull request Mar 7, 2023

Process only Requested Modules when Resolving Imports #5568

Closed

JaroslavTulach mentioned this pull request Mar 10, 2023

Import/Export caches are not along the StdLib #5890

Closed

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

Optimize import/export resolution #5700

Optimize import/export resolution #5700

hubertp commented Feb 20, 2023 •

edited

Loading

JaroslavTulach left a comment

JaroslavTulach Feb 22, 2023

hubertp Feb 28, 2023

JaroslavTulach Feb 22, 2023

JaroslavTulach Mar 1, 2023


		}

		object Cache {

Optimize import/export resolution #5700

Optimize import/export resolution #5700

Conversation

hubertp 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 22, 2023

Choose a reason for hiding this comment

hubertp Feb 28, 2023

Choose a reason for hiding this comment

JaroslavTulach Feb 22, 2023

Choose a reason for hiding this comment

JaroslavTulach Mar 1, 2023

Choose a reason for hiding this comment

hubertp commented Feb 20, 2023 •

edited

Loading