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.
close#8033
Changelog:
- update: run language server initialization once
- fix: issues with async `getSuggestionDatabase` message handling in new IDE
- update: implement unique background jobs
- refactor: initialization logic to Java
- refactor: `UniqueJob` to a marker interface
Unmasked position info for the failed edits so that it is possible to understand what edits are attempted at all. So far unable to locate the problem based solely on the logs and the stacktrace. Added a test confirming that making edits at the end of the file continues to be fine (that's where the problem usually manifests itself).
# Important Notes
References #7978
`AmbgiuousImport` error and `DuplicatedImport` warning must not have `Source` as one of its fields or compiler will crash during a serialization attempt.
Changed the implementation to provide it as a parameter to the `message` method instead.
Fixes#8089
close#7819
Warnings
```
Execution of visualization [...] on value [...] of [Panic] failed.
```
are produced when you are typing an incomplete expression in the component browser and can be misleading.
It seems that Runtime Connector wasn't respecting the protocol it defined itself. The connector should be waiting on the `Api.InitializedNotification` message and only then start forwarding messages. So far it seems this hasn't been a problem, or at least wasn't reported as such, because initialization was fast enough.
Modified `Handler` so that we are certain that its fields hold initialized values when being accessed by different threads.
Should fix problems mentioned in #7898.
It looks like visualization commands had required context lock unnecessairly. Context manager methods are synchronized and therefore should not need the lock before submitting a correspodning background job.
Additionally, the presence of a context lock leads a deadlock:
1. Consider a long running execution of a program that does not finish within the 5 seconds
2. In the meantime there comes either an `AttachVisualization` or `DetachVisualization` request from the client
The latter will get stuck and eventually timeout out because it cannot acquire the lock withing the required time limits. It is still possible that a single long-running `ExecuteJob` might block other ones (including visualization ones) but that's a separate work.
Fixes some issues present in #7941.
# Important Notes
We need to still investigate `ExecuteJob`s need for context lock which might delay the execution of other background jobs that require it as well (mostly concerned about visualization).
