
[API Proposal]: Java API for Lazy Array/Table
MineCake147E opened this issue ยท 6 comments
Background and motivation
Currently, CobaltLuaMachine.toValue
, when it gets a value returned from a @LuaFunction
, it seems to enumerate all elements of a given Map<?, ?>
or Collection<?>
, creating Lua objects for each element.
Running such code in the main thread would hurt tick performance, which leads to a server lag.
I think it's nice to have some additional APIs for cached lazy table/array objects in Java territory, in order to improve performance in the main thread.
API Proposal
package dan200.computercraft.api.lua;
import it.unimi.dsi.fastutil.ints;
public interface ILazyLuaArray {
/*
* Java API for replacing the `__len` metamethod.
* Once this method returns any value, the return value will be cached.
*/
int getLength();
/*
* Java API for replacing the `__pairs` metamethod.
*/
default Iterable<Int2ObjectMap.Entry<Object>> getPairs(ILuaContext context, int index) {
// Default implementation to be supplied
}
/*
* Java API for replacing `__index` metamethod.
* Once this method returns non-null value, the return value will be cached.
*/
@Nullable
Object constructValueAt(ILuaContext context, int index);
}
public interface ILazyLuaTable {
/*
* Java API for replacing the `__pairs` metamethod.
*/
Iterable<Map.Entry<Object, Object>> getPairs(ILuaContext context, int index);
/*
* Java API for replacing `__index` metamethod.
* Once this method returns non-null value, the return value will be cached.
*/
@Nullable
Object constructValueAt(ILuaContext context, Object index);
}
API Usage
/* Intended to be used in something like `InventoryMethods.list()`. */
public final class LazyItemList implements ILazyLuaArray {
Int2ObjectSortedMap<ItemStack> slots;
final int slotCount;
public LazyItemList(IItemHandler inventory) {
var s = inventory.getSlots();
slotCount = s;
slots = new Int2ObjectLinkedOpenHashMap();
for (int i = 0; i < s; i++) {
ItemStack stackInSlot = inventory.getStackInSlot(i);
if (stackInSlot == null || stackInSlot.isEmpty()) continue;
slots.put(i, stackInSlot.copy());
}
}
public int getLength() {
return slotCount;
}
@Nullable
public Object constructValueAt(ILuaContext context, int index) {
var stack = slots.getOrDefault(index, ItemStack.EMPTY);
return !stack.isEmpty() ? VanillaDetailRegistries.ITEM_STACK.getBasicDetails(stack) : null;
}
}
Alternative Designs
constructValueAt
might have to return MethodResult
instead of Object
.
Risks
None
I think turn Map
to LuaTable
lazely is acceptable and may have a little bit performance improvement.
However, make constructValueAt
or getPairs
throws LuaException
is not reasonable. They should not have any side effect but only construct LuaValues lazely.
However, make
constructValueAt
orgetPairs
throwsLuaException
is not reasonable. They should not have any side effect but only construct LuaValues lazely.
I assumed that it was necessary because IDynamicLuaObject
had such function. Correct me if I'm wrong.
However, make
constructValueAt
orgetPairs
throwsLuaException
is not reasonable. They should not have any side effect but only construct LuaValues lazely.I assumed that it was necessary because
IDynamicLuaObject
had such function. Correct me if I'm wrong.
IDynamicLuaObject
's only method that throws LuaException is:
MethodResult callMethod(ILuaContext context, int method, IArguments arguments) throws LuaException;
which only throws error when you call a method. It is reasonable because you are going to do some blackbox operations that may fail internally. But your API proposal is going to throw exceptions while accessing values, for people who do not know the internal logic well, they will feel it really weird since it does not match their common sense.
Just to clarify, in CC:T all returned values are safe to access, because they had already been serialized once.
Your API proposal did nothing wrong on Lua logic. However, If your goal is only improve the performance when accessing, but not do lazy operations that has side effects, then it is better to keep the access from throwing errors.
The main comment I would make is that this seems have a pretty large cost on the Lua agnosticism of CC:T's LuaFunctions (currently the most lua specific things are that tables are associative arrays and that you can return LuaFunctions).
The second one is that I'm just not sure how useful this is. Generally when you are using a peripheral you are using it to get data that isn't owned by the computer, not to perform an expensive calculation (in which case, why not just avoid returning all the junk data in the first place instead of lazy loading it). What would be an actual use case of this feature?
Just to clarify,
CobaltLuaMachine.toValue
shouldn't ever be called on the main thread.
I misunderstood your code. My bad.
- You now need to take defensive copies of everything. For instance, that
stackInSlot.copy()
is probably more expensive for items with NBT (at least on 1.20.1, it'll be better on 1.21) thangetBasicDetails
!
I didn't know that it's so expensive on 1.20. Thank you for telling me that.
Currently,
CobaltLuaMachine.toValue
, when it gets a value returned from a@LuaFunction
, it seems to enumerate all elements of a givenMap<?, ?>
orCollection<?>
, creating Lua objects for each element. Running such code in the main thread would hurt tick performance
Just to clarify, CobaltLuaMachine.toValue
shouldn't ever be called on the main thread. Do you have any profiles where this is causing issues?, I'd definitely be interested in seeing them!
The tricky thing with lazy-loading is that a lot of this work is scheduled on the main thread because it needs to be! If you want to move more work back to the computer thread (with constructValueAt
), then you hit a few problems:
- You're quite limited by what functions you can safely call (for instance, you can call
VanillaDetailRegistries.ITEM_STACK.getBasicDetails
, but notgetDetails
). - You now need to take defensive copies of everything. For instance, that
stackInSlot.copy()
is probably more expensive for items with NBT (at least on 1.20.1, it'll be better on 1.21) thangetBasicDetails
!
I think a more general solution here would probably just be to support arbitrary userdata (dan200/ComputerCraft#173), but have struggled to find a good way to design that which works with CC:T's existing abstractions.