Skip to content

Commit 6f500e6

Browse files
committed
Optimization: Skip ask check where possible
1 parent 4958cda commit 6f500e6

21 files changed

Lines changed: 142 additions & 84 deletions

compiler/shared/src/main/scala/AgentContext.scala

Lines changed: 44 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
package org.nlogo.tortoise.compiler
44

55
import
6-
org.nlogo.core.{ Application, AstNode, Expression, Instruction, ReporterApp, ReporterBlock, SourceLocatable,
7-
Statement, Syntax }
6+
org.nlogo.core.{ Application, AstNode, Expression, Instruction, Reporter, ReporterApp, ReporterBlock,
7+
SourceLocatable, Statement, Syntax }
88

99
import
1010
org.nlogo.core.prim,
@@ -101,24 +101,50 @@ object AgentContext {
101101
s"PrimChecks.context.assertKind($required, '$primName', ${location.start}, ${location.end});\n$bodyJs"
102102
}
103103

104-
// `ask` rejects only the world's own turtles and patches agentsets, and by identity (`agents eq world.turtles`),
105-
// not by syntax. A `let` variable holding `turtles` is rejected too. So the check is needed whenever the
106-
// expression might be holding one of those sets, but an expression that *builds* an agentset, or that reports a
107-
// single agent, provably can't be one. Anything we can't classify still gets the check. -Jeremy B August 2026
104+
// `ask` rejects only the world's own turtles and patches agentsets, and by identity (`agentset == world.turtles()`
105+
// in desktop's `_ask`), not by syntax. A `let` variable holding `turtles` is rejected too. So the check is needed
106+
// whenever the expression might be holding one of those two objects, which rules out two whole classes of
107+
// expression: one that can't report a turtleset or a patchset at all, and one that reports an agentset of its own.
108+
// Anything we can't classify still gets the check. -Jeremy B August 2026
108109
def canBeWholeAgentSet(exp: Expression): Boolean =
110+
if ((agentSetTypeOf(exp) & (Syntax.TurtlesetType | Syntax.PatchsetType)) == 0)
111+
false
112+
else
113+
exp match {
114+
case r: ReporterApp => !reportsAnotherAgentSet(r.reporter)
115+
case _ => true
116+
}
117+
118+
// `one-of` is declared as reporting anything, because for a list it does -- including, in principle, a list holding
119+
// `turtles`. Given an agentset it reports a single agent, and that is the common case worth keeping cheap.
120+
// -Jeremy B September 2026
121+
private def agentSetTypeOf(exp: Expression): Int =
109122
exp match {
110-
case r: ReporterApp =>
111-
r.reporter match {
112-
case _: prim._breed | _: prim.etc._linkbreed | _: prim.etc._links |
113-
_: prim.etc._turtleset | _: prim.etc._patchset | _: prim.etc._linkset |
114-
_: prim._with | _: prim._other | _: prim._neighbors |
115-
_: prim._neighbors4 | _: prim._turtle | _: prim.etc._patch |
116-
_: prim.etc._link | _: prim._oneof | _: prim.etc._myself |
117-
_: prim.etc._self | _: Optimizer._otherwith | _: Optimizer._oneofwith |
118-
_: Optimizer._patchatreporter => false
119-
case _ => true
120-
}
121-
case _ => true
123+
case r: ReporterApp if r.reporter.isInstanceOf[prim._oneof] =>
124+
if (r.args.headOption.exists( (a) => (a.reportedType() & Syntax.ListType) != 0 ))
125+
r.reportedType()
126+
else
127+
Syntax.AgentType
128+
case _ =>
129+
exp.reportedType()
130+
}
131+
132+
// Prims reporting an agentset that is theirs rather than the world's: a set built on the spot, or a stored one (a
133+
// breed, `no-turtles`) that is never the identical object `ask` rejects. Only prims that can report a turtleset or
134+
// a patchset need to be here; the rest are handled by type. -Jeremy B September 2026
135+
private def reportsAnotherAgentSet(reporter: Reporter): Boolean =
136+
reporter match {
137+
case _: prim._breed | _: prim._breedon | _: prim._inradius |
138+
_: prim._neighbors | _: prim._neighbors4 | _: prim._other |
139+
_: prim._turtleson | _: prim._whoarenot | _: prim._with |
140+
_: prim.etc._atpoints | _: prim.etc._bothends | _: prim.etc._breedat |
141+
_: prim.etc._breedhere | _: prim.etc._incone | _: prim.etc._maxnof |
142+
_: prim.etc._inlinkneighbors | _: prim.etc._linkneighbors | _: prim.etc._minnof |
143+
_: prim.etc._nof | _: prim.etc._nopatches | _: prim.etc._noturtles |
144+
_: prim.etc._outlinkneighbors | _: prim.etc._patchset | _: prim.etc._turtlesat |
145+
_: prim.etc._turtleset | _: prim.etc._turtleshere | _: prim.etc._uptonof |
146+
_: prim.etc._withmax | _: prim.etc._withmin | _: Optimizer._otherwith => true
147+
case _ => false
122148
}
123149

124150
// Can code needing `required` run in `context` without a runtime check?

compiler/shared/src/test/scala/AgentContextTest.scala

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,38 @@ class AgentContextTest extends AnyFunSuite {
146146
assertResult(2)(guardCount("to foo [a] ask a [ fd 1 ask link-neighbors [ fd 1 ] ] end"))
147147
}
148148

149+
// Only `world.turtles()` and `world.patches()` themselves are rejected, and by identity, so the check is worth
150+
// emitting only for an expression that could be holding one of them.
151+
152+
test("asking something that could be the whole set is checked") {
153+
assertResult(1)(askGuardCount("to foo [a] ask a [ fd 1 ] end"))
154+
assertResult(1)(askGuardCount("to foo [a] ask turtles [ fd 1 ] fd 1 end"))
155+
assertResult(1)(askGuardCount("to foo [a] ask ifelse-value true [ turtles ] [ a ] [ fd 1 ] end"))
156+
// `one-of` reports a single agent for an agentset, but for a list it reports whatever the list holds.
157+
assertResult(1)(askGuardCount("to foo [a] ask one-of (list turtles patches) [ fd 1 ] end"))
158+
}
159+
160+
test("asking a set the prim built itself is not checked") {
161+
assertResult(0)(askGuardCount("to foo ask turtles-here [ fd 1 ] end"))
162+
assertResult(0)(askGuardCount("to foo ask link-neighbors [ fd 1 ] end"))
163+
assertResult(0)(askGuardCount("to foo ask turtles with [who > 1] [ fd 1 ] end"))
164+
assertResult(0)(askGuardCount("to foo ask n-of 3 turtles [ fd 1 ] end"))
165+
assertResult(0)(askGuardCount("to foo ask other turtles [ fd 1 ] end"))
166+
assertResult(0)(askGuardCount("to foo ask neighbors [ set pcolor red ] end"))
167+
}
168+
169+
test("asking something that isn't a turtleset or a patchset is not checked") {
170+
assertResult(0)(askGuardCount("to foo ask my-links [ set color red ] end"))
171+
assertResult(0)(askGuardCount("to foo ask patch-here [ set pcolor red ] end"))
172+
assertResult(0)(askGuardCount("to foo ask one-of turtles [ fd 1 ] end"))
173+
assertResult(0)(askGuardCount("to foo ask myself [ fd 1 ] end"))
174+
}
175+
176+
private val AskGuardCall = """PrimChecks\.context\.assertAskAllowed\(""".r
177+
178+
private def askGuardCount(code: String): Int =
179+
AskGuardCall.findAllMatchIn(jsFor(code)).length
180+
149181
private val GuardCall = """PrimChecks\.context\.assertKind\((\d+), '([^']+)'""".r
150182

151183
private def guardsIn(code: String): Seq[(String, String)] =

resources/test/dumps/Bacteria Hunt Speeds.js

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

resources/test/dumps/BeeSmart Hive Finding.js

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

resources/test/dumps/Bug Hunt Coevolution.js

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)