jaquadro / StorageDrawers

A mod adding compartmental storage for Minecraft Forge
MIT License
203 stars 160 forks source link

Drawers and Logistics #322

Open Hakameda opened 8 years ago

Hakameda commented 8 years ago

We have a max size drawer cube, 25_25_25 and seem to get some pretty big fps spikes anytime an item goes into or leave the cube.

Any chance of looking into this, Perhaps its the Calculation of removing it with logistics, Or how the drawers themselves search for an item, Perhaps rendering so many?

1 Controller in the very middle, One Slave that pulls items out, One slave for putting items in

2016-07-28_19 08 41

2016-07-28_19 08 33

codewarrior0 commented 8 years ago

This is probably an issue on the side of LogisticsPipes. The drawer slave merely exposes a long list of inventory slots and the logistics network is what is responsible for searching the slots for a place to put an item. There are a few known problems with the way LP searches through the drawer slots:

https://github.com/RS485/LogisticsPipes/issues/936#issuecomment-219266075 https://github.com/jaquadro/StorageDrawers/issues/113#issuecomment-217766377

If you want to gather some performance info, start up Java VisualVM, open the Minecraft process, and set it to "CPU Sampler" while you insert/extract a bunch of items from the slaves. Try to keep it going for a minute or so, then click the Snapshot button and post a screenshot (or better yet, find the Export button on the snapshot and upload the resulting .nps file so we can look at it).

Or since you're on a server, there are a few options for server-side sampling like Opis and WarmRoast.

Hakameda commented 8 years ago

It doesn't seem to cause any tps issues or delays, Only fps drops

codewarrior0 commented 8 years ago

Does it make any difference if you use the Concealment Key on the controller to disable the icon rendering?

Hakameda commented 8 years ago

I'll give that a try

Hakameda commented 8 years ago

Helps a bit, But still fps drops when inserting/removing items with logistics

jaquadro commented 8 years ago

A fair amount of optimization work has already gone into the SD and LP side -- not sure how much more there is to squeeze from it. A max-size cube is a pretty tough thing for any system to deal with, since you're looking at a 15625 slot inventory. That's mainly TPS-related though.

Drawer updates will trigger block updates and re-rendering, so each time an item goes in you're probably looking at a minimum 16x16x16 block area getting re-rendered. Not sure what impact that ultimately has on FPS, but you could test it yourself by placing/breaking some blocks around the structure. I can't really comment on anything that LP might be doing to slow the render loop.

codewarrior0 commented 8 years ago

Drawer updates will trigger block updates and re-rendering

Really? I had the impression that when items are inserted or removed, it only sends a partial update packet for that tile entity, which doesn't cause a chunk re-render.

jaquadro commented 8 years ago

Because of the variable fill indicators which are rendered as a part of the base block, each count update does trigger a render update. This is something that could be improved.

Hakameda commented 8 years ago

If max size can't be optimized anymore, Could it be lowered so people won't make them that big on servers?

Any idea how small it needs to be do minimize the lag when using logistics pipes?

hron84 commented 8 years ago

In smaller sizes, but we experiencing same problems. No tps issues, only fps, even with concealed drawers. Both controller interactions and direct I/o from drawers affected.

Hakameda commented 8 years ago

Perhaps the issue is something else then, If smaller ones also cause issues

AE is considered 1 large inventory isn't it? Don't seem to get same issues with AE/Logistics storage

jaquadro commented 8 years ago

AE is such a different animal, it's difficult to make a comparison. SD is also being handled through its own API which makes things better (at least, I think it should be better than hooking up a wall of diamond chests).

Please do some testing with causing block updates around the storage cube. Does that also cause an impact on FPS?

Hakameda commented 8 years ago

Yes, Removing and placing items near it does So do building/Destroying the drawers

hron84 commented 8 years ago

@jaquadro for me, it causes only FPS drop, no block update problem.

jaquadro commented 8 years ago

If they're concealed, they're not going to cause an FPS problem unless something is causing their chunk to keep re-rendering. Which will be themselves, if items are actively moving in and out of them.

hron84 commented 8 years ago

@jaquadro they're concealed, and yes, only item inserting/extracting causes FPS drop, just hanging out with drawers does not. But I thought if I conceal drawers, it should not cause re-render anymore unless I un-conceal them

jaquadro commented 8 years ago

There's two kinds of rendering:

Storage Drawers is triggering block updates when items enter or leave, because they could change the fill bar. This is something that I should be able to optimize. But if you've got other things nearby like a redstone clock or machines constantly changing state, what I do won't really matter.

hron84 commented 8 years ago

@jaquadro let me rephrase my problem (actually, it is not my problem, but a problem that affecting the base of my friend): with same system if I use a diamond chest instead of a drawer controller (that is the point where AE contacts with the drawers) the FPS drop goes away, no matter how much item enters/leaves the chest (still talking about same setup, nothing changed but drawer controller -> diamond chest). So, the block update (that happens because items enter/leave a diamond chest) still happening, but as a player, I do not experience as serious FPS drop as with functional drawers. If drawer controller goes back, FPS drop appears again.

Basically the configured system is a bit complicated power generating stuff, from growable stuffs. Farms keep producing items, they enters into a drawer system (via AE) and ME system removes items from the storage as the power generator setup needs it. Because the nature of the system, actually does not matter what is being used as a storage, but we decided to do with SD because compacting drawers and much more storing capacity than a bunch of chests (including the option for voiding excess items).

From my POV: in this setup, if I conceal all affected drawers, they should behave same way as a bunch of connected diamond chests work. But they seems like don't.

Hakameda commented 8 years ago

The setup works with an AE buffer in between the drawers and the logistics, Storage bus on the controller then hooked to AE with no fps drops. There was another old mod that used to do a similar thing that maybe could help. Better storage crates i think, One large inventory but seemed to not cause any issue even with updates

jaquadro commented 8 years ago

Give this build a try, and see if it helps with items automatically being inserted/removed causing FPS to drop.

https://github.com/jaquadro/StorageDrawers/releases/tag/sd-1.10.1

codewarrior0 commented 8 years ago

The setup works with an AE buffer in between the drawers and the logistics, Storage bus on the controller then hooked to AE with no fps drops.

This hints that LP is needlessly calling markDirty on the inventory attached to the provider pipe. I found something similar with ExU transfer nodes in another issue: #294

Hakameda commented 8 years ago

@jaquadro I can't test it, Using a mod pack for a server. Can't see until next Beyond Reality update

SonOfTheStars commented 8 years ago

@Hakameda what you could test is if you can see an increase in the number of Chunk Updates. Simply cause alot of Traffic from/to the Drawer System via LP. Press F3 and observe the number of Chunk updates. If its high, @codewarrior0 seems to be right in saying that LP might be needlessly calling markDirty() on the attached inventory.

@codewarrior0 though this might not really be preventable without adding a special case to the Provider/Sink Pipes/Modules. Since the call of markDirty() is needed to prevent wierdness with some types of special inventories.

jaquadro commented 8 years ago

For any inventory handler that calls getStack() on an inventory and modifies that stack directly to set state, markDirty() is crucial for correct behavior. I'm really happy most of this insanity died with the new Forge API in 1.8.9+.

No idea what LP does generally, but the LP/SD bridge actually works through the SD API so markDirty shouldn't be relevant?

SonOfTheStars commented 8 years ago

LP actually appears not to call markDirty() for SD on its own. Sadly we dont have LP for 1.8 so were stuck with the non-unified inventory handling

codewarrior0 commented 8 years ago

What do you think of rate-limiting the redraws from a drawer with an indicator upgrade? I had a very busy drawer in a chunk full of... well, richly-modeled machines, and it killed the FPS of anyone who went near it.

codewarrior0 commented 8 years ago

In fact, since a lot of experts are subscribed to this issue, I'd like to ask:

In 1.10, is it true that markDirty() does not trigger a client sync or a redraw?

SonOfTheStars commented 8 years ago

@codewarrior0 kinda irrelevant, since LP is dead

Hakameda commented 8 years ago

LP will never die! Though the issue still exists on my server. Both AE and LP have same issue. Even with a 5 drawer setup. Seems to be something with io maybe