Skip to content

Commit e4d125d

Browse files
Remove common reasons for materialization to fail due to stack overflows (#539)
This rewrite Val.Obj addSuper and gatherKeys to be iterative instead of recursive. Fixes #392
1 parent a0c0573 commit e4d125d

File tree

5 files changed

+169
-15
lines changed

5 files changed

+169
-15
lines changed

sjsonnet/src/sjsonnet/Val.scala

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,8 @@ object Val {
267267
final class Obj(
268268
val pos: Position,
269269
private var value0: util.LinkedHashMap[String, Obj.Member],
270-
static: Boolean,
271-
triggerAsserts: (Val.Obj, Val.Obj) => Unit,
270+
private val static: Boolean,
271+
private val triggerAsserts: (Val.Obj, Val.Obj) => Unit,
272272
`super`: Obj,
273273
valueCache: util.HashMap[Any, Val] = new util.HashMap[Any, Val](),
274274
private var allKeys: util.LinkedHashMap[String, java.lang.Boolean] = null)
@@ -314,24 +314,41 @@ object Val {
314314
}
315315

316316
def addSuper(pos: Position, lhs: Val.Obj): Val.Obj = {
317-
`super` match {
318-
case null => new Val.Obj(pos, getValue0, false, triggerAsserts, lhs)
319-
case x => new Val.Obj(pos, getValue0, false, triggerAsserts, x.addSuper(pos, lhs))
317+
val objs = mutable.ArrayBuffer(this)
318+
var current = this
319+
while (current.getSuper != null) {
320+
objs += current.getSuper
321+
current = current.getSuper
320322
}
323+
324+
current = lhs
325+
for (s <- objs.reverse) {
326+
current = new Val.Obj(s.pos, s.getValue0, false, s.triggerAsserts, current)
327+
}
328+
current
321329
}
322330

323331
def prettyName = "object"
324332
override def asObj: Val.Obj = this
325333

326334
private def gatherKeys(mapping: util.LinkedHashMap[String, java.lang.Boolean]): Unit = {
327-
if (static) mapping.putAll(allKeys)
328-
else {
329-
if (`super` != null) `super`.gatherKeys(mapping)
330-
getValue0.forEach { (k, m) =>
331-
val vis = m.visibility
332-
if (!mapping.containsKey(k)) mapping.put(k, vis == Visibility.Hidden)
333-
else if (vis == Visibility.Hidden) mapping.put(k, true)
334-
else if (vis == Visibility.Unhide) mapping.put(k, false)
335+
val objs = mutable.ArrayBuffer(this)
336+
var current = this
337+
while (current.getSuper != null) {
338+
objs += current.getSuper
339+
current = current.getSuper
340+
}
341+
342+
for (s <- objs.reverse) {
343+
if (s.static) {
344+
mapping.putAll(s.allKeys)
345+
} else {
346+
s.getValue0.forEach { (k, m) =>
347+
val vis = m.visibility
348+
if (!mapping.containsKey(k)) mapping.put(k, vis == Visibility.Hidden)
349+
else if (vis == Visibility.Hidden) mapping.put(k, true)
350+
else if (vis == Visibility.Unhide) mapping.put(k, false)
351+
}
335352
}
336353
}
337354
}
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
local reduce(func, array, default) =
2+
// assert std.isFunction(func) : error 'reduce(func=...) should be a `function`';
3+
// assert std.isArray(array) : error 'reduce(array=...) should be an `array`';
4+
// assert std.length(array) > 0 : error 'reduce(array=...) should be a non-0 `array`';
5+
local aux(func, array, current, index) =
6+
if index < 0 then
7+
current
8+
else
9+
aux(func, array, func(array[index], current), index - 1) tailstrict;
10+
local length = std.length(array);
11+
if length == 0 then
12+
default
13+
else
14+
aux(func, array, array[length - 1], length - 2);
15+
16+
local visit(
17+
value,
18+
visitor,
19+
ctx,
20+
ctx_reducer,
21+
) =
22+
local new_item = if std.isObject(value) then
23+
local merged = {
24+
[prop]: visit(value[prop], visitor, ctx, ctx_reducer)
25+
for prop in std.objectFields(value)
26+
};
27+
28+
local new_item = {
29+
[prop]: merged[prop].item
30+
for prop in std.objectFields(merged)
31+
};
32+
local new_ctx = reduce(ctx_reducer, std.map(
33+
function(item) item.ctx,
34+
std.objectValues(merged)
35+
), ctx);
36+
37+
{
38+
item: new_item,
39+
ctx: new_ctx,
40+
}
41+
else if std.isArray(value) then
42+
local merged = std.map(
43+
function(item) visit(item, visitor, ctx, ctx_reducer),
44+
value,
45+
);
46+
47+
// If we had a way to tee collections, that would be great... :(
48+
local new_item = std.map(function(item) item.item, merged);
49+
local all_ctx = std.map(function(item) item.ctx, merged);
50+
51+
// Reduce all of the contexts
52+
local new_ctx = reduce(ctx_reducer, all_ctx, ctx);
53+
54+
{
55+
item: new_item,
56+
ctx: new_ctx,
57+
}
58+
else
59+
{
60+
item: value,
61+
ctx: ctx,
62+
};
63+
local replacement = visitor(new_item.ctx, new_item.item);
64+
if replacement != null then
65+
local new_ctx = ctx_reducer(replacement.ctx, new_item.ctx);
66+
67+
{
68+
item: replacement.item,
69+
ctx: new_ctx,
70+
}
71+
else
72+
new_item;
73+
74+
75+
local routes = {
76+
'/inc/users/whoami': {
77+
get: {
78+
responses: {
79+
'200': {
80+
content: {
81+
'application/json': {
82+
schema: {
83+
properties: {
84+
user: {
85+
properties: {
86+
'#id': 'the id this entity',
87+
id: {
88+
format: 'uuid',
89+
type: 'string',
90+
'x-name': 'users.User',
91+
},
92+
},
93+
type: 'object',
94+
},
95+
},
96+
},
97+
},
98+
},
99+
},
100+
},
101+
},
102+
},
103+
};
104+
105+
visit(routes, function(ctx, item) { ctx: ctx, item: item }, {}, function(left, right) left + right)
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
{
2+
"ctx": { },
3+
"item": {
4+
"/inc/users/whoami": {
5+
"get": {
6+
"responses": {
7+
"200": {
8+
"content": {
9+
"application/json": {
10+
"schema": {
11+
"properties": {
12+
"user": {
13+
"properties": {
14+
"#id": "the id this entity",
15+
"id": {
16+
"format": "uuid",
17+
"type": "string",
18+
"x-name": "users.User"
19+
}
20+
},
21+
"type": "object"
22+
}
23+
}
24+
}
25+
}
26+
}
27+
}
28+
}
29+
}
30+
}
31+
}
32+
}

sjsonnet/test/src-js/sjsonnet/FileTests.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ object FileTests extends BaseFileTests {
5555

5656
test("new_test_suite") - {
5757
val t = TestResources_new_test_suite.files.keys.toSeq
58-
.filter(f => f.matches("[^/]+-js\\.jsonnet"))
58+
.filter(f => f.matches("[^/]+-js\\.jsonnet") || (f.matches("[^/]+\\.jsonnet") && !f.contains("-jvm")))
5959
.sorted
6060
assert(t.nonEmpty)
6161
t.foreach { file =>

sjsonnet/test/src-jvm-native/sjsonnet/FileTests.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ object FileTests extends BaseFileTests {
4545
test("new_test_suite") - {
4646
val t = os
4747
.list(testSuiteRoot / "new_test_suite")
48-
.filter(f => f.ext == "jsonnet" && f.last.contains("jvm-native"))
48+
.filter(f => f.ext == "jsonnet" && !f.last.contains("-js"))
4949
assert(t.nonEmpty)
5050
t.foreach { file =>
5151
check(file, "new_test_suite")

0 commit comments

Comments
 (0)