You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by "Magnus Naeslund (JIRA)" <ji...@apache.org> on 2005/07/22 19:19:58 UTC

[jira] Created: (DIRMINA-78) ByteBuffer.wrap((byte[],?) The element of least suprise

ByteBuffer.wrap((byte[],?) The element of least suprise
-------------------------------------------------------

         Key: DIRMINA-78
         URL: http://issues.apache.org/jira/browse/DIRMINA-78
     Project: Directory MINA
        Type: New Feature
    Reporter: Magnus Naeslund
 Assigned to: Trustin Lee 
    Priority: Minor
 Attachments: mina-bytebuffer-wrap-example.diff

*This is just an discussion, not a bug*

The problem:
When mina wraps (buffers or arrays) external data, it keeps push()ing the nio buffer to the stacks of buffers, but it can't reuse them when wrapping  (since they either already exist or was created from nio wrap()).

This could be fixed in two ways: 

1)  When wrapping, re-use buffers as usual, con: extra copy
2) Have a flag that causes the underlying nio buffer to not be pooled after release

I would rather see minas bytebuffer to copy the data than cause an memory leak.
I've just debugged a project that writes byte arrays (old code, needs to write arrays), and found the memory leak that is (kind of) described in the mina ByteBuffer  documentation. I ended up with a unbounded leak of nio HeapByteBuffers.

I know that this is discouraged in the documentation, but I don't like these kind of suprises that you get from using mina without carefully reading the documentation (note that the javadoc warning isn't at the method level).
It's like the close(true) that doesn't flush the buffers before closing the session (I can't understand why anyone wants it the way it is now).

I know copy it's slower, but if you're using wrap(), you'll need to do something like that anyways...
I implemented the copy for byte arrays as an example, because it is the least modification, I'll attach it to this issue.

So after this little rant, what do you think about this suggestion?

Regards,
Magnus Naeslund


Magnus


-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


[jira] Commented: (DIRMINA-78) ByteBuffer.wrap((byte[],?) The element of least suprise

Posted by "Magnus Naeslund (JIRA)" <ji...@apache.org>.
    [ http://issues.apache.org/jira/browse/DIRMINA-78?page=comments#action_12316483 ] 

Magnus Naeslund commented on DIRMINA-78:
----------------------------------------

Maybe i should mention that the patch is totally untested :)

> ByteBuffer.wrap((byte[],?) The element of least suprise
> -------------------------------------------------------
>
>          Key: DIRMINA-78
>          URL: http://issues.apache.org/jira/browse/DIRMINA-78
>      Project: Directory MINA
>         Type: New Feature
>     Reporter: Magnus Naeslund
>     Assignee: Trustin Lee
>     Priority: Minor
>  Attachments: mina-bytebuffer-wrap-example.diff
>
> *This is just an discussion, not a bug*
> The problem:
> When mina wraps (buffers or arrays) external data, it keeps push()ing the nio buffer to the stacks of buffers, but it can't reuse them when wrapping  (since they either already exist or was created from nio wrap()).
> This could be fixed in two ways: 
> 1)  When wrapping, re-use buffers as usual, con: extra copy
> 2) Have a flag that causes the underlying nio buffer to not be pooled after release
> I would rather see minas bytebuffer to copy the data than cause an memory leak.
> I've just debugged a project that writes byte arrays (old code, needs to write arrays), and found the memory leak that is (kind of) described in the mina ByteBuffer  documentation. I ended up with a unbounded leak of nio HeapByteBuffers.
> I know that this is discouraged in the documentation, but I don't like these kind of suprises that you get from using mina without carefully reading the documentation (note that the javadoc warning isn't at the method level).
> It's like the close(true) that doesn't flush the buffers before closing the session (I can't understand why anyone wants it the way it is now).
> I know copy it's slower, but if you're using wrap(), you'll need to do something like that anyways...
> I implemented the copy for byte arrays as an example, because it is the least modification, I'll attach it to this issue.
> So after this little rant, what do you think about this suggestion?
> Regards,
> Magnus Naeslund
> Magnus

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


[jira] Updated: (DIRMINA-78) ByteBuffer.wrap((byte[],?) The element of least suprise

Posted by "Magnus Naeslund (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/DIRMINA-78?page=all ]

Magnus Naeslund updated DIRMINA-78:
-----------------------------------

    Attachment: mina-bytebuffer-wrap-example.diff

Example on solving this, but maybe not the cleanest solution (best might be to not pool it at all)

> ByteBuffer.wrap((byte[],?) The element of least suprise
> -------------------------------------------------------
>
>          Key: DIRMINA-78
>          URL: http://issues.apache.org/jira/browse/DIRMINA-78
>      Project: Directory MINA
>         Type: New Feature
>     Reporter: Magnus Naeslund
>     Assignee: Trustin Lee
>     Priority: Minor
>  Attachments: mina-bytebuffer-wrap-example.diff
>
> *This is just an discussion, not a bug*
> The problem:
> When mina wraps (buffers or arrays) external data, it keeps push()ing the nio buffer to the stacks of buffers, but it can't reuse them when wrapping  (since they either already exist or was created from nio wrap()).
> This could be fixed in two ways: 
> 1)  When wrapping, re-use buffers as usual, con: extra copy
> 2) Have a flag that causes the underlying nio buffer to not be pooled after release
> I would rather see minas bytebuffer to copy the data than cause an memory leak.
> I've just debugged a project that writes byte arrays (old code, needs to write arrays), and found the memory leak that is (kind of) described in the mina ByteBuffer  documentation. I ended up with a unbounded leak of nio HeapByteBuffers.
> I know that this is discouraged in the documentation, but I don't like these kind of suprises that you get from using mina without carefully reading the documentation (note that the javadoc warning isn't at the method level).
> It's like the close(true) that doesn't flush the buffers before closing the session (I can't understand why anyone wants it the way it is now).
> I know copy it's slower, but if you're using wrap(), you'll need to do something like that anyways...
> I implemented the copy for byte arrays as an example, because it is the least modification, I'll attach it to this issue.
> So after this little rant, what do you think about this suggestion?
> Regards,
> Magnus Naeslund
> Magnus

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


[jira] Commented: (DIRMINA-78) ByteBuffer.wrap((byte[],?) The element of least suprise

Posted by "Magnus Naeslund (JIRA)" <ji...@apache.org>.
    [ http://issues.apache.org/jira/browse/DIRMINA-78?page=comments#action_12316600 ] 

Magnus Naeslund commented on DIRMINA-78:
----------------------------------------

Well, the more performant way of doing this would be to have a flag in DefaultByteBuffer that says wether to push the NIO buffer into the stack pool on release().

So when we're using wrap it would still pool the mina bytebuffer container as we do now, but release() would not  shove the NIO buffer back into the pool.

> ByteBuffer.wrap((byte[],?) The element of least suprise
> -------------------------------------------------------
>
>          Key: DIRMINA-78
>          URL: http://issues.apache.org/jira/browse/DIRMINA-78
>      Project: Directory MINA
>         Type: New Feature
>     Reporter: Magnus Naeslund
>     Assignee: Trustin Lee
>     Priority: Minor
>  Attachments: mina-bytebuffer-wrap-example.diff
>
> *This is just an discussion, not a bug*
> The problem:
> When mina wraps (buffers or arrays) external data, it keeps push()ing the nio buffer to the stacks of buffers, but it can't reuse them when wrapping  (since they either already exist or was created from nio wrap()).
> This could be fixed in two ways: 
> 1)  When wrapping, re-use buffers as usual, con: extra copy
> 2) Have a flag that causes the underlying nio buffer to not be pooled after release
> I would rather see minas bytebuffer to copy the data than cause an memory leak.
> I've just debugged a project that writes byte arrays (old code, needs to write arrays), and found the memory leak that is (kind of) described in the mina ByteBuffer  documentation. I ended up with a unbounded leak of nio HeapByteBuffers.
> I know that this is discouraged in the documentation, but I don't like these kind of suprises that you get from using mina without carefully reading the documentation (note that the javadoc warning isn't at the method level).
> It's like the close(true) that doesn't flush the buffers before closing the session (I can't understand why anyone wants it the way it is now).
> I know copy it's slower, but if you're using wrap(), you'll need to do something like that anyways...
> I implemented the copy for byte arrays as an example, because it is the least modification, I'll attach it to this issue.
> So after this little rant, what do you think about this suggestion?
> Regards,
> Magnus Naeslund
> Magnus

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


[jira] Updated: (DIRMINA-78) ByteBuffer.wrap((byte[],?) The element of least suprise

Posted by "Trustin Lee (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/DIRMINA-78?page=all ]

Trustin Lee updated DIRMINA-78:
-------------------------------

    Fix Version: 0.7.4
        Version: 0.7.3
                 0.7.2
                 0.7.1
                 0.7

> ByteBuffer.wrap((byte[],?) The element of least suprise
> -------------------------------------------------------
>
>          Key: DIRMINA-78
>          URL: http://issues.apache.org/jira/browse/DIRMINA-78
>      Project: Directory MINA
>         Type: New Feature
>     Versions: 0.7.3, 0.7.2, 0.7.1, 0.7
>     Reporter: Magnus Naeslund
>     Assignee: Trustin Lee
>     Priority: Minor
>      Fix For: 0.7.4
>  Attachments: mina-bytebuffer-wrap-example.diff, mina-bytebuffer-wrap-fix-1.diff
>
> *This is just an discussion, not a bug*
> The problem:
> When mina wraps (buffers or arrays) external data, it keeps push()ing the nio buffer to the stacks of buffers, but it can't reuse them when wrapping  (since they either already exist or was created from nio wrap()).
> This could be fixed in two ways: 
> 1)  When wrapping, re-use buffers as usual, con: extra copy
> 2) Have a flag that causes the underlying nio buffer to not be pooled after release
> I would rather see minas bytebuffer to copy the data than cause an memory leak.
> I've just debugged a project that writes byte arrays (old code, needs to write arrays), and found the memory leak that is (kind of) described in the mina ByteBuffer  documentation. I ended up with a unbounded leak of nio HeapByteBuffers.
> I know that this is discouraged in the documentation, but I don't like these kind of suprises that you get from using mina without carefully reading the documentation (note that the javadoc warning isn't at the method level).
> It's like the close(true) that doesn't flush the buffers before closing the session (I can't understand why anyone wants it the way it is now).
> I know copy it's slower, but if you're using wrap(), you'll need to do something like that anyways...
> I implemented the copy for byte arrays as an example, because it is the least modification, I'll attach it to this issue.
> So after this little rant, what do you think about this suggestion?
> Regards,
> Magnus Naeslund
> Magnus

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


[jira] Commented: (DIRMINA-78) ByteBuffer.wrap((byte[],?) The element of least suprise

Posted by "Trustin Lee (JIRA)" <ji...@apache.org>.
    [ http://issues.apache.org/jira/browse/DIRMINA-78?page=comments#action_12316607 ] 

Trustin Lee commented on DIRMINA-78:
------------------------------------

It is a good idea to specify to pool or not to pool existing NIO buffers and byte arrays internally.  I'll try to apply this idea soon, and get back here when done.

> ByteBuffer.wrap((byte[],?) The element of least suprise
> -------------------------------------------------------
>
>          Key: DIRMINA-78
>          URL: http://issues.apache.org/jira/browse/DIRMINA-78
>      Project: Directory MINA
>         Type: New Feature
>     Versions: 0.7.3, 0.7.2, 0.7.1, 0.7
>     Reporter: Magnus Naeslund
>     Assignee: Trustin Lee
>     Priority: Minor
>      Fix For: 0.7.4
>  Attachments: mina-bytebuffer-wrap-example.diff, mina-bytebuffer-wrap-fix-1.diff
>
> *This is just an discussion, not a bug*
> The problem:
> When mina wraps (buffers or arrays) external data, it keeps push()ing the nio buffer to the stacks of buffers, but it can't reuse them when wrapping  (since they either already exist or was created from nio wrap()).
> This could be fixed in two ways: 
> 1)  When wrapping, re-use buffers as usual, con: extra copy
> 2) Have a flag that causes the underlying nio buffer to not be pooled after release
> I would rather see minas bytebuffer to copy the data than cause an memory leak.
> I've just debugged a project that writes byte arrays (old code, needs to write arrays), and found the memory leak that is (kind of) described in the mina ByteBuffer  documentation. I ended up with a unbounded leak of nio HeapByteBuffers.
> I know that this is discouraged in the documentation, but I don't like these kind of suprises that you get from using mina without carefully reading the documentation (note that the javadoc warning isn't at the method level).
> It's like the close(true) that doesn't flush the buffers before closing the session (I can't understand why anyone wants it the way it is now).
> I know copy it's slower, but if you're using wrap(), you'll need to do something like that anyways...
> I implemented the copy for byte arrays as an example, because it is the least modification, I'll attach it to this issue.
> So after this little rant, what do you think about this suggestion?
> Regards,
> Magnus Naeslund
> Magnus

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


[jira] Commented: (DIRMINA-78) ByteBuffer.wrap((byte[],?) The element of least suprise

Posted by "Trustin Lee (JIRA)" <ji...@apache.org>.
    [ http://issues.apache.org/jira/browse/DIRMINA-78?page=comments#action_12316579 ] 

Trustin Lee commented on DIRMINA-78:
------------------------------------

Hi, Magnus.

I agree with current documentation can just let user misuse wrap() methods.  But users will still require legacy wrap() methods for performance reason.  What do you think about renaming existing wrap methods to 'fastwrap' or something else, and add new wrap() methods that is safer.  Any better ideas?



> ByteBuffer.wrap((byte[],?) The element of least suprise
> -------------------------------------------------------
>
>          Key: DIRMINA-78
>          URL: http://issues.apache.org/jira/browse/DIRMINA-78
>      Project: Directory MINA
>         Type: New Feature
>     Reporter: Magnus Naeslund
>     Assignee: Trustin Lee
>     Priority: Minor
>  Attachments: mina-bytebuffer-wrap-example.diff
>
> *This is just an discussion, not a bug*
> The problem:
> When mina wraps (buffers or arrays) external data, it keeps push()ing the nio buffer to the stacks of buffers, but it can't reuse them when wrapping  (since they either already exist or was created from nio wrap()).
> This could be fixed in two ways: 
> 1)  When wrapping, re-use buffers as usual, con: extra copy
> 2) Have a flag that causes the underlying nio buffer to not be pooled after release
> I would rather see minas bytebuffer to copy the data than cause an memory leak.
> I've just debugged a project that writes byte arrays (old code, needs to write arrays), and found the memory leak that is (kind of) described in the mina ByteBuffer  documentation. I ended up with a unbounded leak of nio HeapByteBuffers.
> I know that this is discouraged in the documentation, but I don't like these kind of suprises that you get from using mina without carefully reading the documentation (note that the javadoc warning isn't at the method level).
> It's like the close(true) that doesn't flush the buffers before closing the session (I can't understand why anyone wants it the way it is now).
> I know copy it's slower, but if you're using wrap(), you'll need to do something like that anyways...
> I implemented the copy for byte arrays as an example, because it is the least modification, I'll attach it to this issue.
> So after this little rant, what do you think about this suggestion?
> Regards,
> Magnus Naeslund
> Magnus

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


[jira] Updated: (DIRMINA-78) ByteBuffer.wrap((byte[],?) The element of least suprise

Posted by "Magnus Naeslund (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/DIRMINA-78?page=all ]

Magnus Naeslund updated DIRMINA-78:
-----------------------------------

    Attachment: mina-bytebuffer-wrap-fix-1.diff

I whipped up a patch for showing how we could still have performance with wrapped but no memory leaks.
Totally untested, but it compiles :)

> ByteBuffer.wrap((byte[],?) The element of least suprise
> -------------------------------------------------------
>
>          Key: DIRMINA-78
>          URL: http://issues.apache.org/jira/browse/DIRMINA-78
>      Project: Directory MINA
>         Type: New Feature
>     Reporter: Magnus Naeslund
>     Assignee: Trustin Lee
>     Priority: Minor
>  Attachments: mina-bytebuffer-wrap-example.diff, mina-bytebuffer-wrap-fix-1.diff
>
> *This is just an discussion, not a bug*
> The problem:
> When mina wraps (buffers or arrays) external data, it keeps push()ing the nio buffer to the stacks of buffers, but it can't reuse them when wrapping  (since they either already exist or was created from nio wrap()).
> This could be fixed in two ways: 
> 1)  When wrapping, re-use buffers as usual, con: extra copy
> 2) Have a flag that causes the underlying nio buffer to not be pooled after release
> I would rather see minas bytebuffer to copy the data than cause an memory leak.
> I've just debugged a project that writes byte arrays (old code, needs to write arrays), and found the memory leak that is (kind of) described in the mina ByteBuffer  documentation. I ended up with a unbounded leak of nio HeapByteBuffers.
> I know that this is discouraged in the documentation, but I don't like these kind of suprises that you get from using mina without carefully reading the documentation (note that the javadoc warning isn't at the method level).
> It's like the close(true) that doesn't flush the buffers before closing the session (I can't understand why anyone wants it the way it is now).
> I know copy it's slower, but if you're using wrap(), you'll need to do something like that anyways...
> I implemented the copy for byte arrays as an example, because it is the least modification, I'll attach it to this issue.
> So after this little rant, what do you think about this suggestion?
> Regards,
> Magnus Naeslund
> Magnus

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


[jira] Resolved: (DIRMINA-78) ByteBuffer.wrap((byte[],?) The element of least suprise

Posted by "Trustin Lee (JIRA)" <ji...@apache.org>.
     [ http://issues.apache.org/jira/browse/DIRMINA-78?page=all ]
     
Trustin Lee resolved DIRMINA-78:
--------------------------------

    Resolution: Fixed

I added a property called 'pooled' to MINA ByteBuffer.  This property is 'true' by default except for the case you created the wrapped buffer.  In case of the wrapped ones, it is 'false' by default so that they are not returned to the buffer pool.  Of course you can change this bahavior by calling 'setPooled' method.

I mark this issue as 'resolved' because it resolves all issues addressed here by users who have a concern on it.

> ByteBuffer.wrap((byte[],?) The element of least suprise
> -------------------------------------------------------
>
>          Key: DIRMINA-78
>          URL: http://issues.apache.org/jira/browse/DIRMINA-78
>      Project: Directory MINA
>         Type: New Feature
>     Versions: 0.7.3, 0.7.2, 0.7.1, 0.7
>     Reporter: Magnus Naeslund
>     Assignee: Trustin Lee
>     Priority: Minor
>      Fix For: 0.7.4
>  Attachments: mina-bytebuffer-wrap-example.diff, mina-bytebuffer-wrap-fix-1.diff
>
> *This is just an discussion, not a bug*
> The problem:
> When mina wraps (buffers or arrays) external data, it keeps push()ing the nio buffer to the stacks of buffers, but it can't reuse them when wrapping  (since they either already exist or was created from nio wrap()).
> This could be fixed in two ways: 
> 1)  When wrapping, re-use buffers as usual, con: extra copy
> 2) Have a flag that causes the underlying nio buffer to not be pooled after release
> I would rather see minas bytebuffer to copy the data than cause an memory leak.
> I've just debugged a project that writes byte arrays (old code, needs to write arrays), and found the memory leak that is (kind of) described in the mina ByteBuffer  documentation. I ended up with a unbounded leak of nio HeapByteBuffers.
> I know that this is discouraged in the documentation, but I don't like these kind of suprises that you get from using mina without carefully reading the documentation (note that the javadoc warning isn't at the method level).
> It's like the close(true) that doesn't flush the buffers before closing the session (I can't understand why anyone wants it the way it is now).
> I know copy it's slower, but if you're using wrap(), you'll need to do something like that anyways...
> I implemented the copy for byte arrays as an example, because it is the least modification, I'll attach it to this issue.
> So after this little rant, what do you think about this suggestion?
> Regards,
> Magnus Naeslund
> Magnus

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira