Skip to content

Commit

Permalink
Only send suggestions updates when type changes
Browse files Browse the repository at this point in the history
The change adds an additional field to `ExpressionUpdates` messages sent
by `ProgramExecutionSupport` to indicate if the type of value (or its
method pointer) has changed and therefore would potentially require a
suggestions' update.

Prior to #3729 that check was done during the instrumentation. However
we still want to continue to support "pending expression" functionality
therefore `SuggestionsHandler` will use the additional information to
filter only the required expression updates.

Most of the changes are related to adapting our tests to the new field.

Closes #6707.
  • Loading branch information
hubertp committed May 18, 2023
1 parent 7bf9d36 commit 8980672
Show file tree
Hide file tree
Showing 11 changed files with 347 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ final class SuggestionsHandler(
updates.map(u => (u.expressionId, u.expressionType))
)
val types = updates.toSeq
.filter(_.typeChanged)
.flatMap(update => update.expressionType.map(update.expressionId -> _))
suggestionsRepo
.updateAll(types)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class ContextEventsListenerSpec
Some(methodPointer),
Vector(),
false,
true,
Api.ExpressionUpdate.Payload.Value()
)
)
Expand Down Expand Up @@ -100,6 +101,7 @@ class ContextEventsListenerSpec
None,
Vector(),
false,
true,
Api.ExpressionUpdate.Payload.DataflowError(
Seq(Suggestions.function.externalId.get)
)
Expand Down Expand Up @@ -139,6 +141,7 @@ class ContextEventsListenerSpec
None,
Vector(),
false,
false,
Api.ExpressionUpdate.Payload.Panic("Method failure", Seq())
)
)
Expand Down Expand Up @@ -177,6 +180,7 @@ class ContextEventsListenerSpec
None,
Vector(),
false,
false,
Api.ExpressionUpdate.Payload.Value()
)
)
Expand All @@ -191,6 +195,7 @@ class ContextEventsListenerSpec
None,
Vector(),
false,
false,
Api.ExpressionUpdate.Payload.Value()
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ object Runtime {
methodCall: Option[MethodPointer],
profilingInfo: Vector[ProfilingInfo],
fromCache: Boolean,
typeChanged: Boolean,
payload: ExpressionUpdate.Payload
)
object ExpressionUpdate {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1363,23 +1363,29 @@ class RuntimeErrorsTest
Api.ExpressionUpdate.Payload.Panic(
"MyError2",
Seq(xId)
)
),
builtin = false,
typeChanged = false
),
TestMessages.panic(
contextId,
yId,
Api.ExpressionUpdate.Payload.Panic(
"MyError2",
Seq(xId)
)
),
builtin = false,
typeChanged = false
),
TestMessages.panic(
contextId,
mainResId,
Api.ExpressionUpdate.Payload.Panic(
"MyError2",
Seq(xId)
)
),
builtin = false,
typeChanged = false
),
context.executionComplete(contextId)
)
Expand Down Expand Up @@ -1492,10 +1498,14 @@ class RuntimeErrorsTest
contextId,
xId,
ConstantsGen.INTEGER,
Api.MethodPointer(moduleName, moduleName, "foo")
Api.MethodPointer(moduleName, moduleName, "foo"),
fromCache = false,
typeChanged = true
),
TestMessages.update(contextId, yId, ConstantsGen.INTEGER),
TestMessages.update(contextId, mainResId, ConstantsGen.NOTHING),
TestMessages
.update(contextId, yId, ConstantsGen.INTEGER, typeChanged = true),
TestMessages
.update(contextId, mainResId, ConstantsGen.NOTHING, typeChanged = true),
context.executionComplete(contextId)
)
context.consumeOut shouldEqual List("3")
Expand Down Expand Up @@ -1667,9 +1677,12 @@ class RuntimeErrorsTest
contextId,
xId,
ConstantsGen.INTEGER,
Api.MethodPointer(moduleName, moduleName, "foo")
Api.MethodPointer(moduleName, moduleName, "foo"),
fromCache = false,
typeChanged = true
),
TestMessages.update(contextId, yId, ConstantsGen.INTEGER),
TestMessages
.update(contextId, yId, ConstantsGen.INTEGER, typeChanged = true),
context.executionComplete(contextId)
)
context.consumeOut shouldEqual List("3")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ class RuntimeExecutionEnvironmentTest
Some(IF_ENABLED_METH_PTR),
Vector(Api.ProfilingInfo.ExecutionTime(0)),
false,
true,
Api.ExpressionUpdate.Payload.Value()
)
)
Expand Down Expand Up @@ -320,6 +321,7 @@ class RuntimeExecutionEnvironmentTest
Some(IF_ENABLED_METH_PTR),
Vector(Api.ProfilingInfo.ExecutionTime(0)),
false,
true,
Api.ExpressionUpdate.Payload.Value()
)
)
Expand Down
Loading

0 comments on commit 8980672

Please sign in to comment.