Add furniController.lua #37

2020-05-13T20:02:39-04:00

drucifer-sc commented

2020-05-13 20:02:39 -04:00

(Migrated from github.com)

Cheap fix to stop cascading storage updates from eating up all the working time available after failing exports, this needs to be fleshed out better, if the chest is full, scan the slots for any that match and are not full, stop blindly exporting.

Added furniController to miloApps, a multi-furnace-array controller, read the header notes for information

Cheap fix to stop cascading storage updates from eating up all the working time available after failing exports, this needs to be fleshed out better, if the chest is full, scan the slots for any that match and are not full, stop blindly exporting. Added furniController to miloApps, a multi-furnace-array controller, read the header notes for information

kepler155c commented

2020-05-13 23:34:19 -04:00

(Migrated from github.com)

This looks good to me - Anavrins - let me know if you have any comments.

I don't have much time to test out Milo changes. But if you get a chance, would you be able to test this change in storage.lua ? I saw that while reviewing your perf change.

Line 518 of storage.lua (basically - why try pushing other slots if the first one failed ?)

if amount > 0 then
	amount = transfer(key, amount, slot)
	if amount > 0 then
		source.lastUpdate = os.clock()
		target.lastUpdate = os.clock()
	else
		break
	end
end
`

This looks good to me - Anavrins - let me know if you have any comments. I don't have much time to test out Milo changes. But if you get a chance, would you be able to test this change in storage.lua ? I saw that while reviewing your perf change. Line 518 of storage.lua (basically - why try pushing other slots if the first one failed ?) ``` if amount > 0 then amount = transfer(key, amount, slot) if amount > 0 then source.lastUpdate = os.clock() target.lastUpdate = os.clock() else break end end `

drucifer-sc commented

2020-05-14 02:25:00 -04:00

(Migrated from github.com)

Actually trying to avoid the cache update on line 528 altogether (which if I'm reading the code correctly, precedes the fix you're asking me to try), figured it'd be easier accomplished in the exportTask plugin in the first place, saving erroneous calls to export.

kepler155c commented

2020-05-16 12:02:49 -04:00

(Migrated from github.com)

I pushed a fix to address the export performance - the logic in storage.lua was pushing way too many times when the target was full. Therefore, we should not need the proposed change to exportTask.lua. Please review the commit I pushed for this fix.

drucifer-sc commented

2020-05-16 21:51:16 -04:00

(Migrated from github.com)

I applied your update of the storage API, and removed my fix from the affected Milo. The export task average time went straight from ~50ms with my fix only to ~1500ms, but is no longer growing exponentially. I still firmly believe that exportTask needs to be reworked in more detail to prevent unnecessary calls to Storage:export in the first place avoiding these perf hits altogether. I recognize my fix is in the wrong place and is very vague, I will see about reworking it myself.

kepler155c commented

2020-05-17 00:48:24 -04:00

(Migrated from github.com)

With the change i put in, the total execution time should not be hitting 1500. :(

The execution time should be calculated as:
50ms * number of unique items you are exporting to that single chest

If we are exceeding that amount, then possibly we are activating Squid's cost system multiplier (I have no idea how the cost system works).

Going through the storage/caching code is not an issue - it's extremely fast - the biggest factor is waiting for the plethora reply for every request (50ms per call).

I'll try out the change you provided - and I'll do some testing myself.

With the change i put in, the total execution time should not be hitting 1500. :( The execution time should be calculated as: 50ms * number of unique items you are exporting to that single chest If we are exceeding that amount, then possibly we are activating Squid's cost system multiplier (I have no idea how the cost system works). Going through the storage/caching code is not an issue - it's extremely fast - the biggest factor is waiting for the plethora reply for every request (50ms per call). I'll try out the change you provided - and I'll do some testing myself.

drucifer-sc commented

2020-05-17 01:14:53 -04:00

(Migrated from github.com)

Bear in mind for testing, I'm currently exporting to all slots [*] 21-22 unique items on a standard-sized chest (27-slot). This is likely what's causing the problem in the first place as it's looping through trying to export some of each of those 22 items in it's filter list. There's also another 4 chests with single-items being exported to again, all slots [*], so 25-26 individual items being exported, the majority against a single chest however. (I don't care what goes in when, just that it goes in eventually.)

Bear in mind for testing, I'm currently exporting to all slots [\*] 21-22 unique items on a standard-sized chest (27-slot). This is likely what's causing the problem in the first place as it's looping through trying to export some of each of those 22 items in it's filter list. There's also another 4 chests with single-items being exported to again, all slots [\*], so 25-26 individual items being exported, the majority against a single chest however. (I don't care what goes in when, just that it goes in eventually.)

drucifer-sc commented

2020-05-18 04:20:23 -04:00

(Migrated from github.com)

Made my last update to exportTask, should button up the exports. With my fix in place, export times settled in around 200ms idle for 5 chests, which I guess is to be expected? The only qualm I have right now is my use of node.cacheList and I'm contemplating moving it to local cache instead to avoid contaminating the storage config at all.

Ran quick perf test comparison:
Before your storage.lua fix: Exponentially scaling proc time
Clean install of Milo (rm /sys, pastebin run uzghlbnc), with your fix: ~1400-1500ms proc time
My last version of exportTask.lua plus your storage api fix: 200ms or less idle; Lookin' good now!

Actually very broken, would not export smaller portions to fill, only large chunks of whatever was available at the time until all slots were populated, then fail due to bad usage of filterItem. Fixed the code in another revision, and times shot up. I suspect it's all the plethora calls I'm making.

Made my last update to exportTask, should button up the exports. With my fix in place, export times settled in around 200ms idle for 5 chests, which I guess is to be expected? The only qualm I have right now is my use of `node.cacheList` and I'm contemplating moving it to `local cache` instead to avoid contaminating the storage config at all. Ran quick perf test comparison: Before your `storage.lua` fix: Exponentially scaling proc time Clean install of Milo (rm /sys, pastebin run uzghlbnc), with your fix: ~1400-1500ms proc time My last version of `exportTask.lua` plus your storage api fix: 200ms or less idle; Lookin' good now! Actually very broken, would not export smaller portions to fill, only large chunks of whatever was available at the time until all slots were populated, then fail due to bad usage of `filterItem`. Fixed the code in another revision, and times shot up. I suspect it's all the plethora calls I'm making.

kepler155c commented

2020-05-18 13:40:25 -04:00

(Migrated from github.com)

I pushed another change (exportTask.lua) - basically using your concept - please pull this file and give it a shot. Thanks

drucifer-sc commented

2020-05-18 20:20:31 -04:00

(Migrated from github.com)

Much better, exporter task is now sub-100ms with your fix. Might I suggest to increase the accuracy with regards to nbt, that when exporting an item with nbt, on a slot that matches the name and damage, then execute an .getItemMeta() on the slot to check for a matching nbt? Might be worth the little bit of a spike in activity for those unique items, since it'd only be those items. Now how do I retract my edit from the PR?

Much better, exporter task is now sub-100ms with your fix. Might I suggest to increase the accuracy with regards to nbt, that when exporting an item with nbt, on a slot that matches the name and damage, *then* execute an .getItemMeta() on the slot to check for a matching nbt? Might be worth the little bit of a spike in activity for those unique items, since it'd only be those items. Now how do I retract my edit from the PR?

kepler155c commented

2020-05-18 20:44:07 -04:00

(Migrated from github.com)

Thanks for looking into that perf issue - I knew it was bad - but did uncover a lot of extra calls. I think what we have will work for +90% of the cases. The biggest issue is if they are exporting something with unique nbts and the chest is pretty full - this could generate getItemMetas on 20+ slots which will eat up a lot of time - best to just try and push and let that 1 call fail.

To undo the exportTask.lua - do a git checkout [FILE] - that will replace what you have with the current version. Then, just commit again.

Thanks for looking into that perf issue - I knew it was bad - but did uncover a lot of extra calls. I think what we have will work for +90% of the cases. The biggest issue is if they are exporting something with unique nbts and the chest is pretty full - this could generate getItemMetas on 20+ slots which will eat up a lot of time - best to just try and push and let that 1 call fail. To undo the exportTask.lua - do a git checkout [FILE] - that will replace what you have with the current version. Then, just commit again.

drucifer-sc commented

2020-05-18 21:33:01 -04:00

(Migrated from github.com)

Maybe an option in the future to confirm NBT at cost of performance? Maybe an auto-disable recommendation if the process gets really heavy, say proc time of >2s or >97% of total time?

xAnavrins commented

2020-05-18 21:43:14 -04:00

(Migrated from github.com)

0619eee41c broke autocrafting, items would get placed in the inventory and almost imediately get sucked back into the storage.
Removing the break on line 534 fixes it, but that kind of defeats the purpose of removing unessesary peripheral calls though.

0619eee41c31accea72f3c6ee661c1feb4d873f5 broke autocrafting, items would get placed in the inventory and almost imediately get sucked back into the storage. Removing the break on line 534 fixes it, but that kind of defeats the purpose of removing unessesary peripheral calls though.

kepler155c commented

2020-05-18 23:21:34 -04:00

(Migrated from github.com)

I reverted the break -- not sure why that broke anything though ... but need to keep autocrafting working

kepler155c commented

2020-05-18 23:59:43 -04:00

(Migrated from github.com)

Another update to exportTask.lua - I seem to have forgotten that list() does provide NBT info (I must have been thinking about 1.7 or turtles). Hopefully this is it.

drucifer-sc commented

2020-05-23 18:39:38 -04:00

(Migrated from github.com)

This seems to be it. All fixes in place, the system is ~50ms idle on exporter now, spiking up only when there's actual activity.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: MayaTheShy/opus-apps#37