perf(tools): yield when gzip or gunzip #13338
Conversation
github-actions
bot
added
the
cherry-pick kong-ee
chronolaw
changed the title
| count = 0 | ||
| yield() | ||
| end | ||
|
|
||
| local data = input_buffer:get(size) |
There was a problem hiding this comment.
I am thinking whether input_buffer:get() itself can yield if the input_buffer is set to a socket. If it is a yieldable socket, then the new added yield() seems redundant.
Because according to the buffer:set() in https://luajit.org/ext_buffer.html, it supports string and cdata. Is it possible if the buffer is set to a cdata type, which is a socket?
There was a problem hiding this comment.
Currently (in kong gatway), we are using gzip/gunzip only for string data (CP/DP config), so far as I know no socket to be applied to gzip/gunzip.
There was a problem hiding this comment.
Are there any statistics to help determine the optimal gzip-yield-size?
For example, how long does it take to compress data of size GZIP_CHUNK_SIZE * 2?
This would give us a more intuitive sense of the necessary yield intervals.
I checked the yield point, thinking it safe.
|
In @chobits , do you think this data size is a suitable number for now? |
The granularity of ~1ms sounds appropriate. |
| @@ -15,7 +16,16 @@ local GZIP_CHUNK_SIZE = 65535 | |||
|
|
|||
|
|
|||
| local function read_input_buffer(input_buffer) | |||
| local count = 0 | |||
| local yield_size = GZIP_CHUNK_SIZE * 2 | |||
There was a problem hiding this comment.
Is there any consideration to yield for 2 GZIP_CHUNK_SIZE?
There was a problem hiding this comment.
We don't have enough data to support this number, so GZIP_CHUNK_SIZE * 2 is just a estimated value.
|
@ADD-SP , do you think this PR is ready for merge? |
chronolaw
changed the title
|
Successfully created cherry-pick PR for |
Cherry-picked from #13412 Co-authored-by: Chrono <[email protected]>
Summary
KAG-4878
Still in discussion, but I checked the source of ffi-zlib and believe it can yield safely.
Checklist
changelog/unreleased/kongorskip-changeloglabel added on PR if changelog is unnecessary. README.mdIssue reference
Fix #[issue number]