Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Changing orientation of belt tiles introduce big latency #178

Open
baryluk opened this issue Nov 24, 2019 · 4 comments
Open

[BUG] Changing orientation of belt tiles introduce big latency #178

baryluk opened this issue Nov 24, 2019 · 4 comments
Labels
bug Something isn't working

Comments

@baryluk
Copy link

baryluk commented Nov 24, 2019

Describe the bug

Changing orientation using R key is very frequently used tool. It works really fast for inserters, but for belts, underground belts and splitters, it is considerably slower. Maybe beacuse it needs to also look at neighbouring tiles and their orientation and redraw all tiles that are affected.

But latency is visible and can go up to about a half of a second.

To Reproduce

Try attached blueprint. Zoom in, highlight any belt, underground or splitter, and press R. Notice the fps counter in debug mode stop updating for noticable fraction of the time (more than 100ms, but probably less than 500ms).

Looking at Firefox Performance profiler, the event handler invoked when rotating belts and splitters, take about 150-220 ms, which is quit high. The frame rates goes down to 2-3 fps for few frames basically.

Expected behavior

Lower latency, faster response time.

The issue does not happen for the inserters.

Did not test with buildings like chemical plants, storage tanks, or with assemblers with liquid input / output.

Firefox 68.2.0esr.

16x16-3-stage-4x4+4x4+4x4-iterated-clos-network.blueprint.txt

Some smaller, but still considerably blueprints, often don't have this issue, so I suspect some slow data structure (like unsorted array instead of hash table or sorted array) is used for some critical stuff.

Example:

iterated-weaving-4x4.blueprint.txt

@baryluk baryluk added the bug Something isn't working label Nov 24, 2019
@baryluk
Copy link
Author

baryluk commented Nov 24, 2019

Here is some drill down of the costs, that should help.

It looks indeed like there are issues with neighbouring tiles and sorting. They should be optimized.

profile

@baryluk
Copy link
Author

baryluk commented Nov 24, 2019

Actually for underground belts I did observe latency of 850ms on the mentioned blueprint.

This is because it affects more surrounding tiles.

Most of the runtime is spent in sorting and compare function. The compareFn looks simple enough, but maybe there is a way to avoid sorting in the first place? Using R to reorient tiles, belts, splitter and undergrounds, doesn't change position of items, so the result of sorting should be the same.

Also, when figuring out "surrounding area". Maybe the part of the underground belt that is underground should be ignored, and only ends should be considered instead?

@teoxoy
Copy link
Owner

teoxoy commented Nov 28, 2019

I just tested this and it's a lot more noticeable on firefox than any chromium browser - not sure why.

The problem in this case is that the sorting function sorts all the entities in the blueprint every time it runs. It should be optimized to only sort a few tiles away from the newly added sprites.
It also runs on every modified entity instead of once for all modified entities.

@teoxoy
Copy link
Owner

teoxoy commented Mar 11, 2022

I retested this in FF98 and it seems to be considerably faster 200-300ms for underground belts. I haven't looked into optimizing this but I'm not sure if it's worth it.

Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants