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

compression level change with config option #9711

Merged
merged 2 commits into from
Sep 14, 2023

Conversation

Leguan16
Copy link
Contributor

Supersedes #9703

As from the author requested: Here a new version with the correct patches and a config option.

The config option is located under the misc section. I did not know if I should change the version of the config but that should not stop this PR from being merged.

Is it possible to add people as co-authors so the original author also gets credits?

@Leguan16 Leguan16 requested a review from a team as a code owner September 11, 2023 17:48
@lynxplay
Copy link
Contributor

You can generally add a coauthor by simply adding their name into the patch, see e.g. https://github.com/PaperMC/Paper/blob/master/patches/server/0872-Fix-a-bunch-of-vanilla-bugs.patch#L60

@Leguan16
Copy link
Contributor Author

the placement of the option is ok or should I move that as well?

@lynxplay
Copy link
Contributor

the placement of the option is ok or should I move that as well?

yea 👍 imo it is.

@PedroMPagani
Copy link

@Leguan16 good job ;), it'd be nice if paper added on the docs examples of the available compression options with example of rates, as a benchmark of course, this should benefit a few servers a lot with bandwidth decreasing as well as more performance and cost decreasing :)

@lynxplay
Copy link
Contributor

This does require a PR to the docs before it may be merged yea. @Leguan16 if you have the time, it would be great if you can propose the change there as well 👍

@Leguan16
Copy link
Contributor Author

I can add a description to the global-configuration.mdx what it does but I have no time to benchmark this and create a proper docs page for it. Besides, I don't even know how I should benchmark this.
@PedroMPagani you seem to know what it does exactly so what should I put in the description (So I have it correct)?

@lynxplay
Copy link
Contributor

I don't see the usage for benchmarking on the docs. A quick explanation that a higher compression level means more CPU time spend for less data transmitted and a lower value meaning less CPU time spend but more data transmitted is fine.

@Leguan16
Copy link
Contributor Author

Alright! I will add that later.

@Leguan16
Copy link
Contributor Author

I opened a PR in the docs repo
PaperMC/docs#242

@olijeffers0n
Copy link
Member

Docs is ready to go 😄

@lynxplay lynxplay merged commit 7145f41 into PaperMC:master Sep 14, 2023
@Leguan16 Leguan16 deleted the compression-level branch September 14, 2023 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

6 participants