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

Better segment pools #311

Open
2 of 7 tasks
fzhinkin opened this issue May 6, 2024 · 10 comments
Open
2 of 7 tasks

Better segment pools #311

fzhinkin opened this issue May 6, 2024 · 10 comments

Comments

@fzhinkin
Copy link
Collaborator

fzhinkin commented May 6, 2024

Currently, segment pools exist only on JVM (on other platforms, implementations are effectively no-op) and behave more like caches than pools.

There are a few directions in which we may/should develop segment pools:

  • support pools configuration (both in terms of the pool capacity and the size of allocated segments)
  • make sure every segment is returned to a pool once there are no users remaining (Track precise number of shared segment copies to return the last one into a segment pool #347)
  • support unlinking a segment from a pool (for scenarios when we have a byte-buffer backed segment and now sending it via Netty -> segment is no longer in use once we "consumed" it by wrapping into a ByteBuf, but it could not be released as a Netty owns it now)
  • support adding already allocated data into a buffer (related to Missing API: zero-copy wrapping of ByteArray with Source. #166)
  • support pools on other platforms
  • support pool-level isolation (for instance, if there are multiple threads make sure that each of them uses a separate pool, so that data used in one thread would never leak into another thread)
  • support leak tracing (Leak tracing mechanism #144)

This is an epic describing what could be done and tracking progress rather than an instruction to what should be implemented.

@fzhinkin
Copy link
Collaborator Author

The first two points (not returning segments back to the pool and small pool size) are blockers for integration with Ktor (https://youtrack.jetbrains.com/issue/KTOR-6030, ktorio/ktor#4032), so they need to be fixed.

@e5l
Copy link
Member

e5l commented Jun 11, 2024

It would be great to have a system property to adjust the pool size as well

@fzhinkin
Copy link
Collaborator Author

As @bjhham pointed out, Buffer::close should release segments (currently, it's no-op).

@fzhinkin
Copy link
Collaborator Author

However, it could be problematic, as a typical Buffer use scenario is "allocate, use, and forget".

@e5l
Copy link
Member

e5l commented Jun 11, 2024

Maybe we can try to fix this by manual allocation buffer for writing using a different constructor method, so the buffers for channels and primitives in Ktor will be tracked

@fzhinkin fzhinkin self-assigned this Jun 13, 2024
@fzhinkin
Copy link
Collaborator Author

make sure every segment is returned to a pool once there are no users remaining

One of the scenarios when a segment is not returned to the pool is when is was shared between multiple buffers. Currently, it's tracked by a flag, so there's no way to check if a segment can be safely returned back to the pool.

Replacing a flag with a ref-counter solves the issue and the performance impact seems neglectable.

@fzhinkin
Copy link
Collaborator Author

The issue with changing the default pool size is how this property is used: currently, the pool consists of multiple chunks (the number depends on CPU count), and the property applies to each chunk individually. In an unlucky scenario, we may end up with all buckets being filled up with pooled segments, but only one of them will be used.

This place is tricky in terms of performance, I need to research a bit more what could be done.

@e5l
Copy link
Member

e5l commented Jun 13, 2024

You may try using the same strategy as for connection pool with lazy allocation and releasing allocated buffers through time

@fzhinkin
Copy link
Collaborator Author

fzhinkin commented Jul 2, 2024

Currently, I'm gravitating towards a solution with a two-tier segment pool:

  • the first tier remains the same as it is now: a relatively small pool sharded by the thread ID;
  • the second tier will be disabled by default, but once enabled, all failed attempts to take a segment from the first tier will be continue by taking a segment from the second tier before allocating a new segment; the same with segment recycling - if the first tier is full, an attempt to return it to the second tier will be made;
  • unlike the first tier, the second will be shared across all threads.

@fzhinkin
Copy link
Collaborator Author

fzhinkin commented Jul 4, 2024

Some of the issues from the summary are addressed here: #352

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

No branches or pull requests

2 participants