Refactored mutable parts of `ModuleScope` into builder to make it easier to reduce unnecessary locks.
# Important Notes
Elements of ModuleScope (types, imports etc) are used while _building_ of it may still be in progress. In order to make static typing happy, every `ModuleScope.Builder` can be exposed as (unmodifiable) `ModuleScope`.
This change is a re-do of old PR https://github.com/enso-org/enso/pull/8270 that has been abandoned a while ago.
The change attempts to address to case of random `InterruptedExceptions` that can result when a task is interrupted while waiting on a lock. In comparison to the original PR, this change introduces `withXYZLock` methods with a `Callable` parameter. As a result we ensure correct acquisition/release semantics, logging and handling of interruptions.
# Important Notes
https://github.com/enso-org/enso/pull/8270 was too much hassle to rebase against latest develop.
Ensure strict errors are off to prevent unrecoverable errors in IDE.
Also partially reverted a change that prevented application of changes on errors (errors are still logged, as intended).
Closes#10014.
Setting execution environment to the existing one should have no effect.
Should (positively) affect startup in #9789.
# Important Notes
Cancelling jobs and triggering a fresh execute job is expensive and unnecessary, especially on startup, when the result should be the same as before.
Using a fully qualified name for type identifier does not play well when dealing with different classloaders (classes are not recognized as equal/subtypes).
Closes#9876. Likely also affecting #9306.
# Important Notes
There appears to be a number of cases loosely related issues on that subject. Gave up on adding a test cases to simulate the problem but I could no longer reproduce it with LS/runtime on a live project.
```
[org.enso.languageserver.runtime.RuntimeConnector$Endpoint] Failed to deserialize runtime API envelope
com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve type id 'org.enso.polyglot.data.Tree$Node' as a subtype of `org.enso.polyglot.data.Tree$Node<org.enso.polyglot.runtime.Runtime$Api$SuggestionUpdate>`: Not a subtype
at [Source: (byte[])[6718 bytes]; byte offset: #394] (through reference chain: org.enso.polyglot.runtime.Runtime$Api$Response["payload"]->org.enso.polyglot.runtime.Runtime$Api$SuggestionsDatabaseModuleUpdateNotification["updates"]->org.enso.polyglot.data.Tree$Root["children"]->com.fasterxml.jackson.module.scala.deser.GenericFactoryDeserializerResolver$BuilderWrapper[0])
at org.enso.IsolatedClassLoader//com.fasterxml.jackson.databind.exc.InvalidTypeIdException.from(InvalidTypeIdException.java:43)
```
appears to be gone.
We don't seem to run `abortJobs` under a lock, and especially not under the write compilation lock, in other scenarios. This is causing some major slowdown when there is a long running execution or compilation, as currently experienced in the cloud.
This should reduce chances of a timeout.
Also added an option to override the global executor. Currently it would always default to the runtime number of available process which may be suboptimal.
# Important Notes
Pending testing on the impact it will have.
1. Experimenting with invalidating modules' indexes without requiring full write-context locks. That should significantly improve the execution.
2. Improving performance by making background job executor run in a larger threadpool than 1.
`ExecuteJob` can now be interrupted.
We now have a separate threadpool for visualization-related jobs.
# Important Notes
In a lock step situation, a job or command could have been interrupted while waiting for one of the locks. As locks ensured only that they were released once all of them have been acquired this could leave engine in a broken state.
Once `ExecuteJob` could be interrupted this became a blocker as it prevented project startup almost in every case.
The change also makes it careful to avoid constant `ExecuteJob` restarts.
Addresses #9278. There will be follow up work.
The `null` check creates a new Array but always assumed a non-empty one which may lead to
```
java.lang.ArrayIndexOutOfBoundsException: Index 0 out of bounds for length 0
at org.enso.runtime/org.enso.interpreter.service.ExecutionService$FunctionPointer.collectNotAppliedArguments(ExecutionService.java:778)
at org.enso.runtime/org.enso.interpreter.instrument.job.ProgramExecutionSupport$.sendExpressionUpdate(ProgramExecutionSupport.scala:430)
at org.enso.runtime/org.enso.interpreter.instrument.job.ProgramExecutionSupport$.$anonfun$executeProgram$3(ProgramExecutionSupport.scala:81)
at org.enso.runtime/org.enso.interpreter.service.ExecutionCallbacks.callOnComputedCallback(ExecutionCallbacks.java:146)
at
org.enso.runtime/org.enso.interpreter.service.ExecutionCallbacks.updateCachedResult(ExecutionCallbacks.java:117
...
```
Added a guard to prevent the exception. The flag will be useless anyway as we won't enter the for-loop in this case.
Appears to be introduced via #8743. Discovered while debugging #9389.
close#8965
Changelog:
- update: keep a single `ExecuteExpressionJob` in the queue
- update: make `ExecuteExpressionCommand` synchronous to preserve the order of commands
- refactor: separate data structures for `Visualization` and `OneshotExpression` to simplify the logic
related #8689
Fixes a race between the language server SQL updating logic and the engine `DeserializeLibrarySuggestionsJob`s when the library suggestions may start loading before the database is properly cleaned up after the reconnect.
related #8689
Fixes the NPE during the serialization of update messages.
```
java.lang.NullPointerException: Cannot invoke "java.util.UUID.toString()" because "a" is null
```
Uniqueness check of `UpsertVisualizationJob` only involved expressionId. Apparently now GUI sends mutliple visualizations for the same expressions and expects all of them to exist. Since previously we would cancel duplicate jobs, this was problematic.
This change makes sure that uniqueness also takes into account visualization id. Fixed a few logs that were not passing arguments properly.
Closes#8801
# Important Notes
I have not noticed any more problems with loading visualizations so the issue appears to be resolved with this change.
Added a unit test case that would previously fail due to cancellation of a job that upserts visualization.
This is a quick fix to a long standing problem of
`org.enso.interpreter.service.error.FailedToApplyEditsException` which would prevent backend from processing any more changes, rendering GUI (and backend) virtually useless.
Edits are submitted for (background) processing in the order they are handled. However the order of execution of such tasks is not guaranteed. Most of the time edits are processed in the same order as their requests but when they don't, files get quickly out of sync.
Related to #8770.
# Important Notes
I'm not a fan of this change because it essentially blocks all open/file requests until all edits are processed and we already have logic to deal with that appropriately. Moreover those tasks can and should be processed independently. Since we already had the single thread executor present to ensure correct synchronization of open/file/push commands, we are simply adding edit commands to the list.
Ideally we want to have a specialized executor that executes tasks within the same group sequentially but groups of tasks can be executed in parallel, thus ensuring sufficient throughput. The latter will take much longer and will require significant rewrite of the command execution.
Added tests that would previously fail due to non-deterministic execution.
Fixes#8710 by making sure suspended atom fields support works also for "normal" `Atom` instances without any special `Layout`. Moves all _atom related_ classes into single package and hides as much of classes as possible by making them _package private_.
Implements `Warnings.get_all wrap_errors=True` which wraps warnings attached to values inside vectors with `Map_Error`, which includes the position of the value within the vector. See [the documentation](https://github.com/enso-org/enso/blob/develop/docs/semantics/wrapped-errors.md) for more details.
`get_all wrap_errors=True` does not change the warnings that are attached to values -- it wraps them before returning them to the caller, but does not change the original warnings attached to the values.
Wrapped warnings only appear attached to the vector itself. The values inside the vector do not have their warnings wrapped.
Warning propagation is not changed at all; `Warnings.get_all` (with default `wrap_errors=False`) behaves as before. `get_all wrap_errors=True` is meant to be used primarily by the IDE, although it can be used anywhere this wrapping is desired.
close#8663
Changelog:
- update: use `MethodRootNode` for the atom constructor function to preserve the call info in runtime
- fix: return function schema for atom constructors
close#7184
The constructor value was not accessible because during the re-compilation a new instance of the type was registered in runtime. Then during the execution, an old cached instance of the type was used in method resolution.
Changelog:
- update: the registration of types in runtime
- update: invalidate cached nodes that became a resolution error after applying the edit
We infer old IDs from module's IRs location, instead. This workarounds ill-constructed IdMaps at the start of the project.
If IDE or backend ends up with invalid ID map, I suppose application of edits might still fail.
# Important Notes
Closes#8500 by not doing the parsing at all. Since I couldn't figure why we had invalid metadata in the first place, this might still bite us at some point.
Add a local clone of javaFormatter plugin. The upstream is not maintained anymore. And we need to update it to use the newest Google java formatter because the old one, that we use, cannot format sources with Java 8+ syntax.
# Important Notes
Update to Google java formatter 1.18.1 - https://github.com/google/google-java-format/releases/tag/v1.18.1
Evaluating visualization expression may trigger a full compilation. A change in #7042 went a bit too far and led to a situation when there could be compilations running at the same time leading to a rather obscure `RedefinedMethodException` when the compilation on one thread already finished. This will make the logic correct again at the price of potentially slowing the processing of visualization.
Closes#8296.
# Important Notes
Should make visualizations a bit more stable as well.
Encountered a random NPE when playing with bookclubs. Test case demonstrating the problem is attached.
Threw in a bunch of minor tweaks to logs to make life of the person debugging code more pleasant.
Attaching or modifying a visualizations returns early on, to avoid a situation when a background job is stalled (by other jobs) and eventually the request timeouts.
This has an unfortunate consequence that any error reported in the `UpsertVisualizationJob` cannot be reported as a directly reply to a request because the sender has already been removed from the list.
Added more logs to discover why we get errors in the first place.
Modified the API a bit so that we carry `VisualizationContext` instead of three parameters all over the place.
Bonus:
Modified `JsonRpcServerTestKit` to implicitly require a position so that we get better error reporting on failures.
Fixes a random crash (*) during instrumentation. Notice how `onTailCallReturn` calls `onReturnValue` with `null` frame.
Bonus: noticed that for some reason we weren't getting logs for `ExecutionService`. This turned out to be the problem with the logger name which by default was `[enso]` not
`[enso.org.enso.interpreter.service.ExecutionService`] and there is some logic there that normalizes the name and assumed a dot after `enso`. This change fixes the logic.
(*)
```
[enso.org.enso.interpreter.service.ExecutionService] Execution of function main failed (Cannot invoke "com.oracle.truffle.api.frame.VirtualFrame.materialize()" because "frame" is null).
java.lang.NullPointerException: Cannot invoke "com.oracle.truffle.api.frame.VirtualFrame.materialize()" because "frame" is null
at org.enso.interpreter.instrument.IdExecutionInstrument$IdEventNodeFactory$IdExecutionEventNode.onReturnValue(IdExecutionInstrument.java:246)
at org.enso.interpreter.instrument.IdExecutionInstrument$IdEventNodeFactory$IdExecutionEventNode.onTailCallReturn(IdExecutionInstrument.java:274)
at org.enso.interpreter.instrument.IdExecutionInstrument$IdEventNodeFactory$IdExecutionEventNode.onReturnExceptional(IdExecutionInstrument.java:258)
at org.graalvm.truffle/com.oracle.truffle.api.instrumentation.ProbeNode$EventProviderChainNode.innerOnReturnExceptional(ProbeNode.java:1395)
at org.graalvm.truffle/com.oracle.truffle.api.instrumentation.ProbeNode$EventChainNode.onReturnExceptional(ProbeNode.java:1031)
at org.graalvm.truffle/com.oracle.truffle.api.instrumentation.ProbeNode.onReturnExceptionalOrUnwind(ProbeNode.java:296)
at org.enso.interpreter.node.ExpressionNodeWrapper.executeGeneric(ExpressionNodeWrapper.java:119)
at org.enso.interpreter.node.ClosureRootNode.execute(ClosureRootNode.java:85)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.executeRootNode(OptimizedCallTarget.java:718)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.profiledPERoot(OptimizedCallTarget.java:641)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.callBoundary(OptimizedCallTarget.java:574)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.doInvoke(OptimizedCallTarget.java:558)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.callDirect(OptimizedCallTarget.java:504)
at jdk.internal.vm.compiler/org.graalvm.compiler.truffle.runtime.OptimizedDirectCallNode.call(OptimizedDirectCallNode.java:69)
at org.enso.interpreter.node.callable.thunk.ThunkExecutorNode.doCached(ThunkExecutorNode.java:69)
at org.enso.interpreter.node.callable.thunk.ThunkExecutorNodeGen.executeAndSpecialize(ThunkExecutorNodeGen.java:207)
at org.enso.interpreter.node.callable.thunk.ThunkExecutorNodeGen.executeThunk(ThunkExecutorNodeGen.java:167)
...
```
# Important Notes
Fixes regressions introduced in #8148 and #8162
* OpenFileCmd sends a reply when finished
Lack of reply and therefore a non-determinism on when OpenFile handler
can finish, led to some sporadic instability. Once caches format got
changed, things were taking such a long time that I wasn't able to start
even a basic project (requests would start to timeout).
The change also removes PushContextCmd from synchronous cmds (as
introduced in #798 to remove initialization problems in tests as well as
in real scenarions); the change did the job but was also a bit
controversial.
The change can also help with randomly failing applies (#8174) as IDE kept
closing and opening the project that might have exploited the
race-condition.
* Adapt tests
* Make tests more resilient to out of order messages
* Drop retries that lead to confusing errors
* less random failures
* s/OpenFileNotification/OpenFileRequest
Fixes#8186 by turning `IllegalStateException` into log message. Re-assigning of `BindingsMap` can happen in the IDE where evaluation of modules is repeated again and again. In addition to that avoid dropping errors in compiler without them being noticed.
Towards reduced reliance on Scala semantics.
Translated IR.scala to IR.java and extracted implicits that now need to be imported explicitly.
# Important Notes
1:1 translation. For now `@Identifier` and `@ExternalID` represent the old type aliases but are not verified at compile time.
This is because in a mixed Scala/Java world this seems impossible to employ such frameworks as Checker.
