reuse allocated buffer #271
Conversation
|
@tylitianrui have u meet a memory leak issue? how it happens? |
Codecov Report
@@ Coverage Diff @@
## master #271 +/- ##
==========================================
- Coverage 67.44% 67.24% -0.21%
==========================================
Files 26 26
Lines 2666 2674 +8
==========================================
Hits 1798 1798
- Misses 649 657 +8
Partials 219 219
Continue to review full report at Codecov.
|
encode.go
Outdated
| @@ -50,7 +50,13 @@ func NewEncoder() *Encoder { | |||
|
|
|||
| // Clean clean the Encoder (room) for a new object encoding. | |||
| func (e *Encoder) Clean() { | |||
| buffer := make([]byte, 64) | |||
There was a problem hiding this comment.
@wongoo Making a slice of byte have an advantage: avoiding memory leak caused by growth of underlying array . but making it , when call method Clean every time, is unnecessary too. the underlying array can be reused.
this PR, have two advantages:
- reusing the buffer (reducing memory-allocation);
- avoiding memory leak caused by growth of underlying array
|
@tylitianrui In concurrent scenario, an encoder encodes an object and gets a byte buffer, but async to process it. If u reuse the buffer, it may cause the data buffer being changed by other encoding process. It may be better to create a new API, like |
YES, There is a risk of the buffer changed by other one |
encode.go
Outdated
| // it reuse allocated buffer and reduce memory-allocation. | ||
| func (e *Encoder) ReuseBufferClean() { | ||
| var buffer []byte | ||
| if len(e.buffer) <= 128 { |
There was a problem hiding this comment.
does it need to check the buffer length?
There was a problem hiding this comment.
i am sorry, i made a typo. cap , not len.
if not check the buffer capacity. there is a risk of memory leak. for example, an encoder encodes an huge object （eg: 100K ）, the allocated room of buffer is also so big. generally, most of object is not such huge. it causes most of buffer room is not used and not GC . it is
128 may be too small for most situation，i suggest to change it to 512，or allow user to chang it by ‘hessian.SetReuseBufferSize(int)’
good idea
|
128 may be too small for most situation，i suggest to change it to 512，or allow user to chang it by ‘hessian.SetReuseBufferSize(int)’ |
* reuse buffer avoid allocate * reuse buffer clean * typo * reuse buf size 512
What this PR does:
reuse allocated buffer when buffer size < 128 byte. it not only reuse allocated buffer , but also avoid memory leak.