Hex Casting

Hex Casting

6M Downloads

Stack/serialization limit is weirdly enforced

object-Object opened this issue ยท 2 comments

commented

I've mentioned this a few times in Discord, but I don't think an issue has been opened.

The stack/serialization/iota limit, which is used both when serializing iotas and after each frame evaluation in the CastingVM, is checked here:

public static boolean isTooLargeToSerialize(Iterable<Iota> examinee) {
return isTooLargeToSerialize(examinee, 1);
}
private static boolean isTooLargeToSerialize(Iterable<Iota> examinee, int startingCount) {
// We don't recurse here, just a work queue (or work stack, if we liked.)
// Each element is a found sub-iota, and how deep it is.
//
// TODO: is it worth trying to cache the depth and size statically on a SpellList.
var listsToExamine = new ArrayDeque<>(Collections.singleton(new Pair<>(examinee, 0)));
int totalEltsFound = startingCount; // count the first list
while (!listsToExamine.isEmpty()) {
var iotaPair = listsToExamine.removeFirst();
var sublist = iotaPair.getFirst();
int depth = iotaPair.getSecond();
for (var iota : sublist) {
totalEltsFound += iota.size();
if (totalEltsFound >= HexIotaTypes.MAX_SERIALIZATION_TOTAL) {
return true; // too bad
}
var subIotas = iota.subIotas();
if (subIotas != null) {
if (depth + 1 >= HexIotaTypes.MAX_SERIALIZATION_DEPTH) {
return true;
}
listsToExamine.addLast(new Pair<>(subIotas, depth + 1));
}
}
}
// we made it!
return false;
}

HexIotaTypes.MAX_SERIALIZATION_TOTAL is currently hardcoded to 1024. However, that's not the actual limit in practice, for two reasons:

  • isTooLargeToSerialize starts counting at 1, because its initial purpose was to check the size of lists, and we include the list itself in the count. However, this still applies when checking the size of the entire stack, reducing the actual maximum legal stack size to 1023.
  • The MAX_SERIALIZATION_TOTAL and MAX_SERIALIZATION_DEPTH checks use >=, not >, reducing the actual maximum legal stack size to 1022.

This also means that the largest possible list could only contain 1021 iotas.

I'm not sure if this counts as a bug really, but it seems weird.

commented

I agree this is kind of weird. I'd like to revisit this and (like @walksanatora suggests) consider making this configurable

commented

might I also request that we make this configurable (please ๐Ÿ™๐Ÿป)