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

Reduce pipeline v2 caching #401

Merged
merged 1 commit into from
Jun 20, 2023
Merged

Reduce pipeline v2 caching #401

merged 1 commit into from
Jun 20, 2023

Conversation

astrsh
Copy link
Member

@astrsh astrsh commented Jun 20, 2023

Pull Request

Brief description.

Reduces the amount of cached pipeline chunks from 1024 to 64, and reduces the target size of chunks from 500 to 128. This represents an approximate reduction from a 16k x 16k block area to a more sensible 1024 x 1024 block area per pipeline provider loaded by the server.

What Issues Does This Fix?

Fixes excessive memory usage, reported by @ccorp2002 on discord.

Licensing

  • I am the original author of this code, and I am willing to release it
    under GPLv3.
  • I am not the original author of this code, but it is in public domain or
    released under GPLv3 or a
    compatible license.

Types of changes

  • Bug Fix
  • Build system
  • Documentation
  • New Feature
  • Performance
  • Refactoring
  • Repository
  • Revert
  • Style
  • Tests
  • Translation

Compatiblity

  • Breaking
    change
  • Non-Breaking change.

Contribution Guidelines.

  • I have read
    the CONTRIBUTING.md
    document in the root of the git repository.
  • My code follows the code style for this
    project.

Documentation

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Testing

  • I have added tests to cover my changes.
  • All new and existing tests passed.

@astrsh astrsh merged commit 0be7213 into master Jun 20, 2023
@astrsh astrsh deleted the dev/reduce-pipeline-caching branch June 20, 2023 00:18
@solonovamax
Copy link
Contributor

has it been measured what the original memory usage was from this, and how much this causes it to decrease by?

Also, this should be a config option

@astrsh
Copy link
Member Author

astrsh commented Jun 20, 2023

From C_Corp's report on discord, there was ~12gb worth of PipelineBiome[] instances, with a decrease down to 9gb by simply reducing cache size by some amount (not sure what exactly he decreased it by) - usage will vary by how many packs are installed but it probably shouldn't be that high.

Wasn't too scientific with picking numbers, just figured a lower default would be better for now, then revisit it if and or when it becomes an issue (unlikely it will be until more packs that use pipeline v2 such as OW2 are more prevalent), or memory usage is critically examined. Cache size could probably even be smaller, cache hits I believe only tend to happen in a fairly local area, a larger cache would still be somewhat beneficial for the speed of /locate I would think but this would need to be profiled.

Already opened #402 regarding it being a config option.

@solonovamax
Copy link
Contributor

yeah, this definitely needs to be profiled further & cache hits/misses need to be recorded to see how large we should try and make it for optimal performance/memory tradeoff

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.

2 participants