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

Add TPS From Chunk or Location #232

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Euphillya
Copy link

Currently, apart from using the dev-bundle, we have no way to check the TPS. Therefore, I propose implementing it with either the Location or the Chunk as an argument

+ io.papermc.paper.threadedregions.ThreadedRegionizer.ThreadedRegion<io.papermc.paper.threadedregions.TickRegions.TickRegionData, io.papermc.paper.threadedregions.TickRegions.TickRegionSectionData>
+ region = world.regioniser.getRegionAtSynchronised(x, z);
+ if (region == null) {
+ return new double[]{ 20.0, 20.0, 20.0, 20.0, 20.0 };
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good behavior, and it's totally undocumented, maybe change to Optional<double[]>/double @Nullable [] or throw and error?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified it with a Nullable, which I think is the best option.

+ } else {
+ io.papermc.paper.threadedregions.TickRegions.TickRegionData regionData = region.getData();
+ return new double[] {
+ regionData.getRegionSchedulingHandle().getTickReport5s(System.nanoTime()).tpsData().segmentAll().average(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to at least store the time in nano seconds? As i remember it's not the fastest method to be called so many times

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is already some internal storage of this, because typing tps will show it from the top regions. Would propose to consult that if possible.
imagem

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to at least store the time in nano seconds? As i remember it's not the fastest method to be called so many times

Yes, calling it every time is not a good idea, so I preferred to store it in a variable (like /tps)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants