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

Avoid whole source reparsing when the IDE performs a simple edit #3508

Merged
merged 67 commits into from
Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
cecd735
IncrementalUpdatesTest is a copy of RuntimeServerTest written in Java
JaroslavTulach Jun 1, 2022
8532a9a
Recognize simple TextEdit change
JaroslavTulach Jun 2, 2022
9aeeb2e
Eliminating useless Scala <-> Java collection conversions
JaroslavTulach Jun 2, 2022
fd7f238
Use node counting instrument to fail when reparsing happens
JaroslavTulach Jun 3, 2022
ccce1fd
No need to create a dummy node. Return null
JaroslavTulach Jun 3, 2022
8bd50ab
Identify simple edit and apply it to right Truffle node
JaroslavTulach Jun 3, 2022
fab4189
Hack to treat IDE's simple+comment edit as a simple edit
JaroslavTulach Jun 3, 2022
467fb19
Applying javafmt
JaroslavTulach Jun 4, 2022
6303000
Associating TruffleFile, Rope, Source into an immutable record and up…
JaroslavTulach Jun 4, 2022
1a87a6d
Update SourceSections when performing simple edit
JaroslavTulach Jun 5, 2022
1164679
Survive when changed value isn't a number
JaroslavTulach Jun 7, 2022
7603648
Formatted by scalafmt
JaroslavTulach Jun 7, 2022
31648ad
Introducing patchable tag to mark nodes that know how to patch themse…
JaroslavTulach Jun 7, 2022
a66862b
Avoid iterating node subtrees that are clearly off
JaroslavTulach Jun 8, 2022
28a162a
Using mapExpressions to update the IR after simple edit
JaroslavTulach Jun 8, 2022
86ef968
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Increm…
JaroslavTulach Jun 8, 2022
40eb24b
receiveNIgnoreStdLib method now requires two arguments
JaroslavTulach Jun 8, 2022
d7deb6c
Module maintains its latest Source as well as 'deltas' against latest…
JaroslavTulach Jun 8, 2022
03b58e4
Note in changelog and using a method reference
JaroslavTulach Jun 9, 2022
69a2f98
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Increm…
JaroslavTulach Jun 9, 2022
45e7bb4
Applying proper formatting with javafmtAll
JaroslavTulach Jun 9, 2022
a785033
Generalizing the test to work also on text literals
JaroslavTulach Jun 9, 2022
6797f59
Handle simple edits of text literals
JaroslavTulach Jun 9, 2022
348c18d
Applying scalafmt formatting
JaroslavTulach Jun 10, 2022
dea5e92
Avoid modifications to ModuleScope
JaroslavTulach Jun 10, 2022
1e4e8a4
Representing interactive state by presence of patchedValues
JaroslavTulach Jun 10, 2022
b97bd2d
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Increm…
JaroslavTulach Jun 10, 2022
48a6c32
Call Module.createSection to adjust to deltas provided by position ma…
JaroslavTulach Jun 12, 2022
f37250c
Associate UnwindException with a value and use it in the IdExecutionI…
JaroslavTulach Jun 12, 2022
eb865ea
Support inline sources different from those associated with a Module
JaroslavTulach Jun 13, 2022
b85e455
Root node without a source produces no source sections
JaroslavTulach Jun 13, 2022
dbe6548
Merging with develop branch
JaroslavTulach Jun 14, 2022
0bdfde9
Enabling use of Frgaal and Truffle in runtime-with-instruments tests
JaroslavTulach Jun 14, 2022
71ff2b6
Moving IncrementalUpdatesTest to runtime-with-instruments project
JaroslavTulach Jun 14, 2022
919ab9d
Working with WeakHashMap shall be done behind a boundary
JaroslavTulach Jun 14, 2022
d78f850
Adding withDebug commands to runtime-with-instruments
JaroslavTulach Jun 14, 2022
a84ae04
Use pattern matching to find SimpleUpdate case class
JaroslavTulach Jun 14, 2022
2cc359e
Proper formatting
JaroslavTulach Jun 14, 2022
d754e16
Update the Truffle AST from a provided IR.Expression
JaroslavTulach Jun 14, 2022
c80567a
Share conversion of number literal to runtime value between parsing a…
JaroslavTulach Jun 14, 2022
c156570
Subsequent updates scan the previously modified nodes for a match
JaroslavTulach Jun 14, 2022
a680c25
Uses SlowToInstrumentTag. RuntimeInstrumentTest 8:3 with should not i…
JaroslavTulach Jun 15, 2022
56131f3
Trading one again failing test for three passing. Score is 10:1
JaroslavTulach Jun 15, 2022
4c039ff
All 11 tests of RuntimeInstrumentTest is passing
JaroslavTulach Jun 15, 2022
41c81e0
Removing LocationFilter
JaroslavTulach Jun 15, 2022
3a10046
Always expect ClosureRootNode from the entryCallTarget
JaroslavTulach Jun 15, 2022
cc94d61
Renamed to AvoidIdInstrumentationTag
JaroslavTulach Jun 15, 2022
4891467
No changes in test needed
JaroslavTulach Jun 15, 2022
03ddb48
Documenting the purpose of PatchedModuleValues
JaroslavTulach Jun 15, 2022
162ed20
Merge branch 'jtulach/RewriteLocationFilter' into wip/jtulach/Increme…
JaroslavTulach Jun 15, 2022
2d9e4ef
Documentation for ModuleSources
JaroslavTulach Jun 15, 2022
8959ba2
Clearing debris and unused imports
JaroslavTulach Jun 15, 2022
ba152bc
Compute simpleUpdate with the help of map and filter
JaroslavTulach Jun 16, 2022
08dbd6f
Javadoc clarification
JaroslavTulach Jun 16, 2022
c8f0228
Javadoc clarification
JaroslavTulach Jun 16, 2022
6c81820
Addressing another round of comments
JaroslavTulach Jun 16, 2022
9240bda
Merge branch 'develop' into wip/jtulach/IncrementalUpdates-182323784
mergify[bot] Jun 16, 2022
52630ed
Connecting documentation of setLiteralSource with PatchedModuleValues
JaroslavTulach Jun 16, 2022
fd67025
Explaining meaning of IndexRange.between(firstFunctionLine, afterFunc…
JaroslavTulach Jun 16, 2022
5d280d5
More information in Javadoc
JaroslavTulach Jun 16, 2022
0a573ed
newIR.map
JaroslavTulach Jun 16, 2022
ec417c8
newIR.map 2
JaroslavTulach Jun 16, 2022
f088163
Underscore
JaroslavTulach Jun 16, 2022
f54d387
_._2.size
JaroslavTulach Jun 16, 2022
7144dbc
Ensure only IR.Literal is returned
JaroslavTulach Jun 16, 2022
a5db906
Throw CompilerError rather than returning a string with error message
JaroslavTulach Jun 16, 2022
68a9edf
Merge branch 'develop' into wip/jtulach/IncrementalUpdates-182323784
JaroslavTulach Jun 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
package org.enso.interpreter.node.expression.literal;

import com.oracle.truffle.api.Assumption;
import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.Truffle;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.nodes.NodeInfo;
import org.enso.interpreter.node.ExpressionNode;

/** A representation of integer literals in Enso. */
@NodeInfo(shortName = "IntegerLiteral")
public final class IntegerLiteralNode extends ExpressionNode {
private final long value;
private static final Assumption CONSTANTS_ARE_CONSTANTS =
Truffle.getRuntime().createAssumption("Constants were never updated");
@CompilerDirectives.CompilationFinal private long value;

private IntegerLiteralNode(long value) {
this.value = value;
Expand All @@ -31,6 +36,19 @@ public static IntegerLiteralNode build(long value) {
*/
@Override
public Object executeGeneric(VirtualFrame frame) {
if (CONSTANTS_ARE_CONSTANTS.isValid()) {
return this.value;
}
return readValue();
}

@CompilerDirectives.TruffleBoundary
private Object readValue() {
return this.value;
}

public void updateConstant(String text) {
this.value = Long.valueOf(text);
CONSTANTS_ARE_CONSTANTS.invalidate();
}
}
140 changes: 107 additions & 33 deletions engine/runtime/src/main/java/org/enso/interpreter/runtime/Module.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.logging.Level;
import org.enso.compiler.ModuleCache;
import org.enso.compiler.core.IR;
import org.enso.compiler.core.IR$Literal$Number;
import org.enso.compiler.phase.StubIrBuilder;
import org.enso.interpreter.node.callable.dispatch.CallOptimiserNode;
import org.enso.interpreter.node.callable.dispatch.LoopingCallOptimiserNode;
Expand All @@ -34,6 +35,7 @@
import org.enso.polyglot.LanguageInfo;
import org.enso.polyglot.MethodNames;
import org.enso.text.buffer.Rope;
import org.enso.text.editing.model;

/** Represents a source module with a known location. */
@ExportLibrary(InteropLibrary.class)
Expand Down Expand Up @@ -77,9 +79,7 @@ public boolean isBefore(CompilationStage stage) {
}

private ModuleScope scope;
private TruffleFile sourceFile;
private Rope literalSource;
private Source cachedSource;
private Sources sources;
private final Package<TruffleFile> pkg;
private CompilationStage compilationStage = CompilationStage.INITIAL;
private boolean isIndexed = false;
Expand All @@ -99,7 +99,7 @@ public boolean isBefore(CompilationStage stage) {
* @param sourceFile the module's source file.
*/
public Module(QualifiedName name, Package<TruffleFile> pkg, TruffleFile sourceFile) {
this.sourceFile = sourceFile;
this.sources = Sources.NONE.newWith(sourceFile);
this.pkg = pkg;
this.name = name;
this.cache = new ModuleCache(this);
Expand All @@ -117,7 +117,7 @@ public Module(QualifiedName name, Package<TruffleFile> pkg, TruffleFile sourceFi
* @param literalSource the module's source.
*/
public Module(QualifiedName name, Package<TruffleFile> pkg, String literalSource) {
this.literalSource = Rope.apply(literalSource);
this.sources = Sources.NONE.newWith(Rope.apply(literalSource), false);
this.pkg = pkg;
this.name = name;
this.cache = new ModuleCache(this);
Expand All @@ -135,7 +135,7 @@ public Module(QualifiedName name, Package<TruffleFile> pkg, String literalSource
* @param literalSource the module's source.
*/
public Module(QualifiedName name, Package<TruffleFile> pkg, Rope literalSource) {
this.literalSource = literalSource;
this.sources = Sources.NONE.newWith(literalSource, false);
this.pkg = pkg;
this.name = name;
this.cache = new ModuleCache(this);
Expand All @@ -152,6 +152,7 @@ public Module(QualifiedName name, Package<TruffleFile> pkg, Rope literalSource)
* belong to a package.
*/
private Module(QualifiedName name, Package<TruffleFile> pkg) {
this.sources = Sources.NONE;
this.name = name;
this.scope = new ModuleScope(this);
this.pkg = pkg;
Expand All @@ -177,14 +178,13 @@ public static Module empty(QualifiedName name, Package<TruffleFile> pkg) {
/** Clears any literal source set for this module. */
public void unsetLiteralSource() {
this.isInteractive = false;
this.literalSource = null;
this.cachedSource = null;
this.sources = Sources.NONE;
this.compilationStage = CompilationStage.INITIAL;
}

/** @return the literal source of this module. */
public Rope getLiteralSource() {
return literalSource;
return sources.literalSource();
}

/**
Expand All @@ -193,18 +193,23 @@ public Rope getLiteralSource() {
* @param source the module source.
*/
public void setLiteralSource(String source) {
setLiteralSource(Rope.apply(source));
setLiteralSource(Rope.apply(source), null, null);
}

/**
* Sets new literal sources for the module.
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved
*
* @param source the module source.
*/
public void setLiteralSource(Rope source) {
this.literalSource = source;
public void setLiteralSource(Rope source, IR$Literal$Number change, model.TextEdit edit) {
if (this.scope != null && edit != null) {
if (this.scope.simpleUpdate(edit)) {
this.sources = this.sources.newWith(source, true);
return;
}
}
this.sources = this.sources.newWith(source, false);
this.compilationStage = CompilationStage.INITIAL;
this.cachedSource = null;
this.isInteractive = true;
}

Expand All @@ -214,19 +219,14 @@ public void setLiteralSource(Rope source) {
* @param file the module source file.
*/
public void setSourceFile(TruffleFile file) {
this.literalSource = null;
this.sourceFile = file;
this.sources = Sources.NONE.newWith(file);
this.compilationStage = CompilationStage.INITIAL;
this.cachedSource = null;
this.isInteractive = false;
}

/** @return the location of this module. */
public String getPath() {
if (sourceFile != null) {
return sourceFile.getPath();
}
return null;
return sources.getPath();
}

/**
Expand Down Expand Up @@ -259,17 +259,13 @@ public void ensureScopeExists() {
* @throws IOException when the source comes from a file that can't be read.
*/
public Source getSource() throws IOException {
if (cachedSource != null) {
return cachedSource;
final Source cached = sources.cachedSource();
if (cached != null) {
return cached;
}
if (literalSource != null) {
cachedSource =
Source.newBuilder(LanguageInfo.ID, literalSource.characters(), name.toString()).build();
} else if (sourceFile != null) {
cachedSource = Source.newBuilder(LanguageInfo.ID, sourceFile).build();
literalSource = Rope.apply(cachedSource.getCharacters().toString());
}
return cachedSource;
Sources newSources = sources.ensureCachedSource(name);
sources = newSources;
return newSources.cachedSource();
}

private void compile(Context context) throws IOException {
Expand Down Expand Up @@ -350,7 +346,7 @@ public void setIndexed(boolean indexed) {

/** @return the source file of this module. */
public TruffleFile getSourceFile() {
return sourceFile;
return sources.sourceFile();
}

/** @return {@code true} if the module is interactive, {@code false} otherwise */
Expand Down Expand Up @@ -434,8 +430,7 @@ private static AtomConstructor getConstructor(ModuleScope scope, Object[] args)
private static Module reparse(Module module, Object[] args, Context context)
throws ArityException {
Types.extractArguments(args);
module.cachedSource = null;
module.literalSource = null;
module.sources = module.sources.newWith(null, false);
module.isInteractive = false;
module.wasLoadedFromCache = false;
try {
Expand Down Expand Up @@ -577,4 +572,83 @@ Object getMembers(boolean includeInternal) {
MethodNames.Module.GET_ASSOCIATED_CONSTRUCTOR,
MethodNames.Module.EVAL_EXPRESSION);
}

private record Sources(
TruffleFile sourceFile,
AdaptiveRope ropeHolder,
Source cachedSource) {

static final Sources NONE = new Sources(null, new AdaptiveRope(null), null);

Sources newWith(TruffleFile f) {
return new Sources(f, ropeHolder(), cachedSource());
}

Sources newWith(Rope r, boolean keepCachedSource) {
if (keepCachedSource) {
if (r.characters().length() != literalSource().characters().length()) {
throw new IllegalStateException();
}
this.ropeHolder.update(r);
return this;
} else {
return new Sources(sourceFile(), new AdaptiveRope(r), null);
}
}

Sources ensureCachedSource(QualifiedName name) throws IOException {
if (cachedSource != null) {
return this;
}
if (literalSource() != null) {
var src = Source.newBuilder(LanguageInfo.ID, ropeHolder, name.toString()).build();
return new Sources(sourceFile, ropeHolder, src);
} else if (sourceFile != null) {
var src = Source.newBuilder(LanguageInfo.ID, sourceFile).build();
var lit = Rope.apply(src.getCharacters().toString());
return new Sources(sourceFile, new AdaptiveRope(lit), src);
}
throw new IllegalStateException();
}

String getPath() {
return sourceFile() == null ? null : sourceFile().getPath();
}

Rope literalSource() {
return ropeHolder.rope;
}

private static final class AdaptiveRope implements CharSequence {
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved
Rope rope;

AdaptiveRope(Rope rope) {
this.rope = rope;
}

void update(Rope r) {
this.rope = r;
}

@Override
public int length() {
return rope.characters().length();
}

@Override
public char charAt(int index) {
return rope.characters().charAt(index);
}

@Override
public CharSequence subSequence(int start, int end) {
return rope.characters().subSequence(start, end);
}

@Override
public String toString() {
return rope.toString();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,17 @@
import java.util.*;

import com.oracle.truffle.api.interop.TruffleObject;
import com.oracle.truffle.api.nodes.Node;
import com.oracle.truffle.api.nodes.NodeUtil;
import com.oracle.truffle.api.nodes.RootNode;
import com.oracle.truffle.api.source.SourceSection;
import org.enso.interpreter.node.expression.literal.IntegerLiteralNode;
import org.enso.interpreter.runtime.Module;
import org.enso.interpreter.runtime.callable.atom.AtomConstructor;
import org.enso.interpreter.runtime.callable.function.Function;
import org.enso.interpreter.runtime.error.RedefinedMethodException;
import org.enso.interpreter.runtime.error.RedefinedConversionException;
import org.enso.text.editing.model;

/** A representation of Enso's per-file top-level scope. */
public class ModuleScope implements TruffleObject {
Expand Down Expand Up @@ -295,4 +301,51 @@ public void reset() {
conversions = new HashMap<>();
polyglotSymbols = new HashMap<>();
}

public boolean simpleUpdate(model.TextEdit edit) {
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved
boolean f1 = updateFunctionsMap(edit, methods.values());
boolean f2 = updateFunctionsMap(edit, conversions.values());
return f1 || f2;
}

private boolean updateFunctionsMap(model.TextEdit edit, Collection<? extends Map<?, Function>> values) {
boolean found = false;
for (Map<?, Function> f : values) {
found |= updateFunctions(edit, f.values());
}
return found;
}

private boolean updateFunctions(model.TextEdit edit, Collection<Function> fns) {
boolean found = false;
for (Function f : fns) {
found |= updateFunction(edit, f);
}
return found;
}

private boolean updateFunction(model.TextEdit edit, Function f) {
return updateNode(edit, f.getCallTarget().getRootNode());
}

private boolean updateNode(model.TextEdit edit, Node node) {
boolean found = false;
if (node instanceof IntegerLiteralNode integerNode) {
SourceSection at = node.getSourceSection();
if (
at != null &&
at.getStartLine() - 1 == edit.range().start().line() &&
at.getStartColumn() - 1 == edit.range().start().character() &&
at.getEndLine() - 1 == edit.range().end().line() &&
at.getEndColumn() == edit.range().end().character()
) {
found = true;
integerNode.updateConstant(edit.text());
}
}
for (Node n : NodeUtil.findNodeChildren(node)) {
found |= updateNode(edit, n);
}
return found;
}
}
Loading