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

Lock Contention issue #291

Open
PedroMPagani opened this issue Sep 16, 2024 · 6 comments
Open

Lock Contention issue #291

PedroMPagani opened this issue Sep 16, 2024 · 6 comments
Labels
status: needs triage type: bug Something doesn't work as it was intended to.

Comments

@PedroMPagani
Copy link

Expected behavior

Have almost no lock overhead that would trigger a possible lag source due to lock await across multiple regions.

I don't know exactly how this could even sort of work, but I feel like this a very bad limitation for the design of Folia currently. if not the most limiting issue. outside just making sure that code is optimized in here, I wonder what other sort of design could be approached.

Observed/Actual behavior

Ok, to start off, the current lock is acquired when it seems that there's not an available section, this shouldn't be an issue if there is one, because that would avoid the contention. But otherwise, the lock is global for the entire design of the system, and if it's within processes end-up costing a lot more, this is basically creating a contention across multiple regions that could potentially for example, be loading a chunk.
since this method is acquired by the addChunk( method.
now let's say that this process for some reason takes 10 milliseconds. that would mean every single region that is also trying to touch the lock, would have to wait 10 millis, and this lock contention happens across the Region Threads, so you could sort of consider this lock a database call in the "main-thread" of Paper to some point.

Another issue

This locker is also used for the markTicking and markNotTicking, which could increase extreme delays, including sort of "lag" delays that people might not be able to visualize fully if they don't have a lot of players which is the focus of Folia, so let's say that a single acquire end-up costing 200ms, this would cause lag on every single region that is also trying to tick, even though the contention on the markTick is just await time, not processing itself.

Steps/models to reproduce

Region merging, players loading chunks, things like this.

Plugin and Datapack List

.

Folia version

Latest 1.21.1 branch

Other

No response

@PedroMPagani PedroMPagani added status: needs triage type: bug Something doesn't work as it was intended to. labels Sep 16, 2024
@PedroMPagani
Copy link
Author

image
Here are some examples of the issue, which will lead to those regions that were locking the lock,to instead take 400ms lag spike each

@PedroMPagani
Copy link
Author

I'm trying to help improve the software. but Since some people seem to care too much about every detail of the pattern, i'll close this.

@Kadeluxe
Copy link

Why would you care about random users comments? If some "maintainer" closes an issue then OK, that's their business. Otherwise I don't see why you would want to close own issues if there were valid reasons to open them at the first place.

@PedroMPagani
Copy link
Author

Why would you care about random users comments? If some "maintainer" closes an issue then OK, that's their business. Otherwise I don't see why you would want to close own issues if there were valid reasons to open them at the first place.

Because that's from the Paper team.

@clrxbl
Copy link
Member

clrxbl commented Sep 19, 2024

Why would you care about random users comments? If some "maintainer" closes an issue then OK, that's their business. Otherwise I don't see why you would want to close own issues if there were valid reasons to open them at the first place.

Because that's from the Paper team.

The person who has replied to you on both Discord & previous Github issues, is in fact, not part of the Paper team.

@PedroMPagani PedroMPagani reopened this Sep 19, 2024
@Spottedleaf
Copy link
Member

Yeah there is some potential gain to be made by using something like ReentrantAreaLock here, however using that would greatly complicate the implementation. It's sort of on my TODO list, but not very high.

You can alleviate some of this overhead by using a larger grid exponent (like 4 or 5).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs triage type: bug Something doesn't work as it was intended to.
Projects
None yet
Development

No branches or pull requests

4 participants