dev@grizzly.java.net

Re: buffer dispose question

From: Tigran Mkrtchyan <tigran.mkrtchyan_at_desy.de>
Date: Thu, 29 Mar 2012 13:52:19 +0200

Hi Alexey.

created http://java.net/jira/browse/GRIZZLY-1241

Tigran.


On Thu, Mar 29, 2012 at 12:10 PM, Oleksiy Stashok
<oleksiy.stashok_at_oracle.com> wrote:
> Hi Tigran,
>
> sorry missed your post.
> inline...
>
>
>
> On 03/28/2012 11:57 AM, Tigran Mkrtchyan wrote:
>>
>> On Wed, Mar 28, 2012 at 11:18 AM, Oleksiy Stashok
>> <oleksiy.stashok_at_oracle.com>  wrote:
>>>
>>> On 03/22/2012 04:51 PM, Tigran Mkrtchyan wrote:
>>>>
>>>> On Thu, Mar 22, 2012 at 4:41 PM, Oleksiy Stashok
>>>> <oleksiy.stashok_at_oracle.com>    wrote:
>>>>>
>>>>> On 03/22/2012 04:32 PM, Tigran Mkrtchyan wrote:
>>>>>>
>>>>>> we'll probably need to introduce another allowBufferDispose method in
>>>>>> CompositeBuffer, which will propagate the dispose flag to underlying
>>>>>> Buffers.
>>>>>>
>>>>>> Now when you dispose CompositeBuffer, if its
>>>>>> allowInternalBuffersDispose
>>>>>> flag is true, it iterates over all the internal Buffers and calls
>>>>>> internalBuffer.tryDispose(). So it's up to each internal Buffer to
>>>>>> decide
>>>>>> if
>>>>>> it wants to be disposed or not.
>>>>>> This is OK if you know that buffer is composite, But I don't (well I
>>>>>> can always ask isComposite).
>>>>>>
>>>>>>        Buffer buffer = xdr.body();
>>>>>>        buffer.allowBufferDispose(true);
>>>>>>         _context.write(_context.getAddress(), buffer, null);
>>>>>>
>>>>>> buffer can be CompositeBuffer of HeapBuffer. This is what grzzly gives
>>>>>> me depending on does message arrived at once or in multiple chinks.
>>>>>
>>>>> AFAIR the Buffer, which represents incoming bytes, has to allow buffer
>>>>> dispose by default, cause we know that this Buffer comes from Grizzly
>>>>> core
>>>>> and we don't have any reference to it.
>>>>> Isn't it the case?
>>>>
>>>> I believe not.
>>>
>>> You're right.
>>> Would you mind to file an issue?
>>
>> Sure. About allowBufferDispose on a composite buffer or on non
>> disposable buffers comming from Grizzly.
>
> IMO propagating allowBufferDispose(...) to internal Buffers might be
> dangerous, and we need some method to allowBufferDispose just for
> CompositeBuffer wrapper. So here I propose to introduce a special method
> (for ex. allowBufferDisposeRecursive) in the CompositeBuffer interface, what
> do you think?
>
> As for the 2nd part "making buffers which coming from Grizzly disposable",
> yes we'd like to fix that.
>
> Thanks.
>
> WBR,
> Alexey.
>
>
>>
>> Tigran.
>>>
>>> Thanks.
>>>
>>> WBR,
>>> Alexey.
>>>
>>>
>>>> Tigran.
>>>>>
>>>>>
>>>>>> Other question:
>>>>>>
>>>>>> In case a buffer is not disposed do I get memory leak or it will be GC
>>>>>> and allocated a new one?
>>>>>
>>>>> It will be GC'ed, the dispose functionality it's like an optimization
>>>>> bonus
>>>>> you may want to use if you know what you do.
>>>>>
>>>>> Thanks.
>>>>>
>>>>> WBR,
>>>>> Alexey.
>>>>>
>>>>>
>>>>>> Tigran.
>>>>>>
>>>>>>> Specifically for CompositeBuffer, allowBufferDispose means whether
>>>>>>> this
>>>>>>> CompositeBuffer instance will or will not be returned to a
>>>>>>> thread-local
>>>>>>> object cache after CompositeBuffer.tryDispose is called.
>>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>> WBR,
>>>>>>> Alexey.
>>>>>>>
>>>>>>>
>>>>>>> On 03/22/2012 04:03 PM, Tigran Mkrtchyan wrote:
>>>>>>>>
>>>>>>>> test case attached
>>>>>>>>
>>>>>>>> On Thu, Mar 22, 2012 at 3:43 PM, Tigran Mkrtchyan
>>>>>>>> <tigran.mkrtchyan_at_desy.de>        wrote:
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I have a following code:
>>>>>>>>>
>>>>>>>>> Buffer buffer1 = ...;
>>>>>>>>> Buffer buffer2 = ...;
>>>>>>>>>
>>>>>>>>>  Buffer composite =
>>>>>>>>> BuffersBuffer.create(MemoryManager.DEFAULT_MEMORY_MANAGER,
>>>>>>>>>                buffer1, buffer2 );
>>>>>>>>>  composite.allowBufferDispose(true);
>>>>>>>>>
>>>>>>>>> At this point I expect that
>>>>>>>>>
>>>>>>>>> composite.tryDispose() and composite.dispose() will dispose buffer1
>>>>>>>>> and
>>>>>>>>> buffer2.
>>>>>>>>>
>>>>>>>>> Nevertheless this is not the case. I think
>>>>>>>>>
>>>>>>>>> composite.allowBufferDispose(true);
>>>>>>>>>
>>>>>>>>> have to propagate to  buffer1 and buffer2 or
>>>>>>>>>
>>>>>>>>> BuffersBuffer.removeAndDisposeBuffers  have to call dispose on
>>>>>>>>> underlying buffers instead of tryDispose.
>>>>>>>>>
>>>>>>>>> Tigran.
>>>>>>>
>>>>>>>
>