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

Buffer pool #22

Closed
flycash opened this issue Sep 21, 2021 · 4 comments · Fixed by #39
Closed

Buffer pool #22

flycash opened this issue Sep 21, 2021 · 4 comments · Fixed by #39
Assignees
Labels
Milestone

Comments

@flycash
Copy link
Contributor

flycash commented Sep 21, 2021

English Only

Please check existing issues first to avoid duplication and answer the questions below before submitting your issue.

Use case(s)

When we are building a query, we can use strings.Builder. But in fact, we should think about reuse the buffer to avoid GC and improve the performance.

Usually, we will use sync.Pool to cache the buffer. But there is a problem that the buffer's capacity may be expanded when it was put back.

For example, the original capacity of buffer is 128, and it means that we can only store 128 bytes. However, we reuse this buffer to build a long query which has more than 128 bytes, e.g. 200 bytes, and then the buffer capacity was expanded to 256 bytes.

If most of the queries only need 50 bytes, it means that we waste a lot memory. A typical solution is avoiding cache big buffer.

Proposed Solution

(Please describe your proposed design/solution, if any)

Alternatives Considered

(Other possible options for solving the problem?)

Additional Context

(Paste any relevant logs - please use code blocks (```) to format console output,
logs, and code as it's very hard to read otherwise.)

(If you can, link to the line of code that might be helpful to explain the context)

@flycash
Copy link
Contributor Author

flycash commented Sep 21, 2021

I think we may need multiple pools. For example, one of them store the buffer < 128 bytes, another stores 128-1024 bytes.
If we use one pool, all the buffer in this pool finally will be expanded to the limitation. For example, if we do not store the buffer > 256bytes, all the buffer in this pool are possibly expanded to 256 bytes..

@flycash flycash added this to the 0.0.1 - CURD milestone Sep 21, 2021
@Codexiaoyi
Copy link
Collaborator

I think we may need multiple pools. For example, one of them store the buffer < 128 bytes, another stores 128-1024 bytes.
If we use one pool, all the buffer in this pool finally will be expanded to the limitation. For example, if we do not store the buffer > 256bytes, all the buffer in this pool are possibly expanded to 256 bytes..

Yes, I think it's necessary. The first version could simply divide pools. In the builder.go file, we return the Query struct, so do we include the size of Args in the calculation? Maybe we can cache the Query.

@flycash
Copy link
Contributor Author

flycash commented Sep 22, 2021

We don't need to think about the size of Args. We only take SQL into consideration.

@Codexiaoyi
Copy link
Collaborator

We are currently using bytebufferpool to complete memory reuse. With issue #39 .

@flycash flycash linked a pull request Oct 22, 2021 that will close this issue
@flycash flycash closed this as completed Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants