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

Extract mutable builder from ModuleScope #9914

Merged
merged 27 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
4da192d
Minor refactorings
hubertp Apr 30, 2024
f2122ec
wip
hubertp May 2, 2024
bfdf093
don't reset types in scopes
hubertp May 9, 2024
2484337
Make builders usage explicit
hubertp May 9, 2024
c49ae59
minor allocation optimizations
hubertp May 10, 2024
e6ec56d
reduce lock scope
hubertp May 10, 2024
201d76e
remove redundant DelayedModuleScope
hubertp May 10, 2024
c7da06b
nits
hubertp May 10, 2024
696d44f
Merge branch 'develop' into wip/hubert/9736-visualizations
hubertp May 24, 2024
a53bf6c
Illustrate when immutable ModulesScope is created
hubertp May 24, 2024
08c13ee
Introduce ImportExportScope proxy
hubertp May 28, 2024
d88d27d
Ditch `get_unresolved_symbol_scope`
hubertp May 28, 2024
1f347d0
Drop built/build check for ModuleScope.Builder
hubertp Jun 2, 2024
1f61c3f
ModuleScope.reset() no more
hubertp Jun 3, 2024
02117cf
cleanup
hubertp Jun 3, 2024
10c48be
drop aux methods by delegating to proxy
hubertp Jun 3, 2024
209c4a3
Ensure elements of ModuleScope are immutable
hubertp Jun 4, 2024
6cd3c36
Merge branch 'develop' into wip/hubert/9736-visualizations
hubertp Jun 4, 2024
d5f811c
Adapt tests
hubertp Jun 4, 2024
ffbeb95
nit
hubertp Jun 4, 2024
be6330f
PR review
hubertp Jun 4, 2024
aa1286e
Type can only have a reference to Module
hubertp Jun 5, 2024
6b4a208
Drop caching of views
hubertp Jun 5, 2024
7cf8099
PR comments
hubertp Jun 5, 2024
790c0d8
Merge branch 'develop' into wip/hubert/9736-visualizations
hubertp Jun 5, 2024
93d5d69
fix compilation issue
hubertp Jun 5, 2024
ed875eb
nit
hubertp Jun 5, 2024
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
23 changes: 3 additions & 20 deletions distribution/lib/Standard/Base/0.0.0-dev/src/Meta.enso
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ type Unresolved_Symbol
- new_name: The new name for the unresolved symbol.
rename : Text -> Any
rename self new_name =
create_unresolved_symbol new_name self.scope
create_unresolved_symbol new_name self.value

## ADVANCED
GROUP Metadata
Expand All @@ -219,14 +219,6 @@ type Unresolved_Symbol
name : Text
name self = get_unresolved_symbol_name self.value

## ADVANCED
GROUP Metadata
ICON metadata

Returns the definition scope of an unresolved symbol.
scope : Any
scope self = get_unresolved_symbol_scope self.value

## ADVANCED
GROUP Metadata
ICON metadata
Expand Down Expand Up @@ -348,9 +340,9 @@ get_polyglot_language value = @Builtin_Method "Meta.get_polyglot_language"

Arguments:
- name: The name of the unresolved symbol.
- scope: The scope in which the symbol name is unresolved.
- symbol: The unresolved symbol from which to get the scope.
create_unresolved_symbol : Text -> Any -> Unresolved_Symbol
create_unresolved_symbol name scope = @Builtin_Method "Meta.create_unresolved_symbol"
create_unresolved_symbol name symbol = @Builtin_Method "Meta.create_unresolved_symbol"

## PRIVATE

Expand All @@ -361,15 +353,6 @@ create_unresolved_symbol name scope = @Builtin_Method "Meta.create_unresolved_sy
get_unresolved_symbol_name : Unresolved_Symbol -> Text
get_unresolved_symbol_name symbol = @Builtin_Method "Meta.get_unresolved_symbol_name"

## PRIVATE

Obtains the scope in which the provided unresolved symbol was created.

Arguments:
- symbol: The unresolved symbol from which to get the scope.
get_unresolved_symbol_scope : Unresolved_Symbol -> Any
get_unresolved_symbol_scope symbol = @Builtin_Method "Meta.get_unresolved_symbol_scope"

## PRIVATE

Get the fields of an atom constructor.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ class ModuleManagementTest
|""".stripMargin)

mainModule.reparse()
val mainFun2 = mainModule.getMethod(assocCons, "main").get
val assocCons2 = mainModule.getAssociatedType
val mainFun2 = mainModule.getMethod(assocCons2, "main").get
mainFun2.execute().asLong() shouldEqual 4567L
}

Expand All @@ -88,21 +89,24 @@ class ModuleManagementTest
mainModule.setSource("""
|main = 456
|""".stripMargin)
val mainFun2 = mainModule.getMethod(assocCons, "main").get
val assocCons2 = mainModule.getAssociatedType
val mainFun2 = mainModule.getMethod(assocCons2, "main").get
mainFun2.execute().asLong() shouldEqual 456L

mainModule.setSource("""
|main = 789
|""".stripMargin)
val mainFun3 = mainModule.getMethod(assocCons, "main").get
val assocCons3 = mainModule.getAssociatedType
val mainFun3 = mainModule.getMethod(assocCons3, "main").get
mainFun3.execute().asLong() shouldEqual 789L

ctx.writeMain("""
|main = 987
|""".stripMargin)

mainModule.setSourceFile(ctx.pkg.mainFile.getAbsolutePath)
val mainFun4 = mainModule.getMethod(assocCons, "main").get
val assocCons4 = mainModule.getAssociatedType
val mainFun4 = mainModule.getMethod(assocCons4, "main").get
mainFun4.execute().asLong() shouldEqual 987L
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ static Set<String> localVarNames(int localVarsCnt) {
* Creates a main method, generates some local variables, and fills their identifiers in the given
* set.
*
* @param localVarsCnt How many local variables should be initialized in the main method
* @param localVarNames local variables that should be initialized in the main method
* @return Body of the main method
*/
static InlineSource createMainMethodWithLocalVars(Context ctx, Set<String> localVarNames)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,12 @@ RuntimeException formatDiagnostic(

// Truffle related

void truffleRunCodegen(Module module, CompilerConfig config) throws IOException;
void truffleRunCodegen(Module module, ModuleScopeBuilder scopeBuilder, CompilerConfig config)
throws IOException;

// module related

void runStubsGenerator(Module module);
void runStubsGenerator(Module module, ModuleScopeBuilder scopeBuilder);

boolean typeContainsValues(String name);

Expand Down Expand Up @@ -147,5 +148,11 @@ public abstract static class Module {
public abstract org.enso.compiler.core.ir.Module getIr();

public abstract boolean isPrivate();

public abstract ModuleScopeBuilder getScopeBuilder();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing getScopeBuilder() would make the system more predictable.


public abstract ModuleScopeBuilder newScopeBuilder();
}

public abstract static class ModuleScopeBuilder {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -360,24 +360,29 @@ class Compiler(

runErrorHandling(requiredModules)

requiredModules.foreach { module =>
val requiredModulesWithScope = requiredModules.map { module =>
if (
!context
.getCompilationStage(module)
.isAtLeast(
CompilationStage.AFTER_RUNTIME_STUBS
)
) {
context.runStubsGenerator(module)
val moduleScopeBuilder = module.getScopeBuilder()
context.runStubsGenerator(module, moduleScopeBuilder)
context.updateModule(
module,
{ u =>
u.compilationStage(CompilationStage.AFTER_RUNTIME_STUBS)
}
)
(module, moduleScopeBuilder)
} else {
(module, module.getScopeBuilder)
}
}
requiredModules.foreach { module =>

requiredModulesWithScope.foreach { case (module, moduleScopeBuilder) =>
if (
!context
.getCompilationStage(module)
Expand All @@ -393,7 +398,7 @@ class Compiler(
context.getModuleName(module)
)

context.truffleRunCodegen(module, config)
context.truffleRunCodegen(module, moduleScopeBuilder, config)
}
context.updateModule(
module,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,10 @@ public FunctionCallInstrumentationNode.FunctionCall prepareFunctionCall(
Module module, String typeName, String methodName)
throws TypeNotFoundException, MethodNotFoundException {
ModuleScope scope = module.compileScope(context);
Type type =
scope
.getType(typeName)
.orElseThrow(() -> new TypeNotFoundException(module.getName().toString(), typeName));
Type type = scope.getType(typeName, false);
if (type == null) {
throw new TypeNotFoundException(module.getName().toString(), typeName);
}
Function function = scope.lookupMethodDefinition(type, methodName);
if (function == null) {
throw new MethodNotFoundException(module.getName().toString(), type, methodName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,70 +63,66 @@ class UpsertVisualizationJob(
ctx.locking.withContextLock(
config.executionContextId,
this.getClass,
() =>
ctx.locking.withWriteCompilationLock(
this.getClass,
() => {
val maybeCallable =
UpsertVisualizationJob.evaluateVisualizationExpression(
config.visualizationModule,
config.expression
)

maybeCallable match {
case Left(ModuleNotFound(moduleName)) =>
ctx.endpoint.sendToClient(
Api.Response(Api.ModuleNotFound(moduleName))
)
None
() => {
hubertp marked this conversation as resolved.
Show resolved Hide resolved
val maybeCallable =
UpsertVisualizationJob.evaluateVisualizationExpression(
config.visualizationModule,
config.expression
)

case Left(EvaluationFailed(message, result)) =>
replyWithExpressionFailedError(
maybeCallable match {
case Left(ModuleNotFound(moduleName)) =>
ctx.endpoint.sendToClient(
Api.Response(Api.ModuleNotFound(moduleName))
)
None

case Left(EvaluationFailed(message, result)) =>
replyWithExpressionFailedError(
config.executionContextId,
visualizationId,
expressionId,
message,
result
)
None

case Right(EvaluationResult(module, callable, arguments)) =>
val visualization =
UpsertVisualizationJob.updateAttachedVisualization(
visualizationId,
expressionId,
module,
config,
callable,
arguments
)
val stack =
ctx.contextManager.getStack(config.executionContextId)
val runtimeCache = stack.headOption
.flatMap(frame => Option(frame.cache))
val cachedValue = runtimeCache
.flatMap(c => Option(c.get(expressionId)))
UpsertVisualizationJob.requireVisualizationSynchronization(
stack,
expressionId
)
cachedValue match {
case Some(value) =>
ProgramExecutionSupport.executeAndSendVisualizationUpdate(
config.executionContextId,
visualizationId,
runtimeCache.getOrElse(new RuntimeCache),
stack.headOption.get.syncState,
visualization,
expressionId,
message,
result
value
)
None

case Right(EvaluationResult(module, callable, arguments)) =>
val visualization =
UpsertVisualizationJob.updateAttachedVisualization(
visualizationId,
expressionId,
module,
config,
callable,
arguments
)
val stack =
ctx.contextManager.getStack(config.executionContextId)
val runtimeCache = stack.headOption
.flatMap(frame => Option(frame.cache))
val cachedValue = runtimeCache
.flatMap(c => Option(c.get(expressionId)))
UpsertVisualizationJob.requireVisualizationSynchronization(
stack,
expressionId
)
cachedValue match {
case Some(value) =>
ProgramExecutionSupport.executeAndSendVisualizationUpdate(
config.executionContextId,
runtimeCache.getOrElse(new RuntimeCache),
stack.headOption.get.syncState,
visualization,
expressionId,
value
)
None
case None =>
Some(Executable(config.executionContextId, stack))
}
case None =>
Some(Executable(config.executionContextId, stack))
}
}
)
}
}
)
}

Expand Down Expand Up @@ -284,7 +280,6 @@ object UpsertVisualizationJob {
): Either[EvaluationFailure, AnyRef] = {
Either
.catchNonFatal {
ctx.locking.assertWriteCompilationLock()
ctx.executionService.evaluateExpression(module, argumentExpression)
}
.leftFlatMap {
Expand Down Expand Up @@ -355,7 +350,6 @@ object UpsertVisualizationJob {
.catchNonFatal {
expression match {
case Api.VisualizationExpression.Text(_, expression, _) =>
ctx.locking.assertWriteCompilationLock()
ctx.executionService.evaluateExpression(
expressionModule,
expression
Expand Down Expand Up @@ -504,7 +498,10 @@ object UpsertVisualizationJob {
callback,
arguments
)
invalidateCaches(visualization)
ctx.locking.withWriteCompilationLock(
this.getClass,
() => invalidateCaches(visualization)
)
ctx.contextManager.upsertVisualization(
visualizationConfig.executionContextId,
visualization
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public interface IR {
* @return the external identifier for this IR node
*/
default Option<@ExternalID UUID> getExternalId() {
return location().flatMap(l -> l.id().map(id -> id));
return location().flatMap(l -> l.id());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ object CallArgument {
|""".toSingleLine

/** @inheritdoc */
override def children: List[IR] = name.toList :+ value
override def children: List[IR] =
name.map(List(_, value)).getOrElse(List(value))

/** @inheritdoc */
override def showCode(indent: Int): String = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,10 @@ protected ExecutableNode parse(InlineParsingRequest request) throws InlineParsin
var m = org.enso.interpreter.runtime.Module.fromCompilerModule(mod);
var toTruffle =
new IrToTruffle(
context, request.getSource(), m.getScope(), redirectConfigWithStrictErrors);
context,
request.getSource(),
m.getScopeBuilder(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should request newScopeBuilder(), fill it and build it (install it) to the Module.

redirectConfigWithStrictErrors);
exprNode = toTruffle.runInline(ir, sco, "<inline_source>");
} else {
exprNode = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.stream.Stream;
import org.enso.compiler.phase.ImportResolverAlgorithm;
import org.enso.editions.LibraryName;
import org.enso.interpreter.runtime.EnsoContext;
Expand Down Expand Up @@ -74,10 +73,8 @@ protected List<String> hiddenNames(Object ex) {
}

@Override
protected List<Type> definedEntities(UnresolvedSymbol name) {
var associatedType = module.getScope().getType(name.getName());
var allRelativeTypes = module.getScope().getTypes().values();
return Stream.concat(associatedType.stream(), allRelativeTypes.stream()).toList();
protected List<Type> definedEntities(UnresolvedSymbol symbol) {
return module.getScope().getAllTypes(symbol.getName());
}

@Override
Expand Down
Loading
Loading