Make derived buffers recyclable / Add missing overrides to ByteBuf impls #5128

trustin · 2016-04-13T06:03:58Z

Motivation:

slice(), duplicate() and readSlice() currently create a non-recyclable
derived buffer instance. Under heavy load, an application that creates a
lot of derived buffers can put the garbage collector under pressure.

Modifications:

Deprecate the old derived buffer implementations
Add the new recyclable derived buffer implementations, which has its
own reference count value
- When a derived buffer is created, its internal reference count is 0.
- When retain() is called on a derived buffer, the internal reference
  count becomes a positive value and the original buffer's retain() is
  called.
- When release() is called on a derived buffer, the internal reference
  count is decreased first, and then the original buffer's release()
  is called when the internal reference count is 0.
Add ByteBufUtil.duplicate/slice() so that a user can easily implement
ByteBuf.duplicate/slice()
Add missing overrides in some ByteBuf impls
Fix incorrect range checks in SlicedByteBuf
Miscellaneous:
- Merge Duplicated/SlicedAbstractByteBuf.unwrap0() into unwrap() using
  covariant return type

Result:

Derived buffers are now recycled when retained and released, although
they are not recycled if a user called release() against the original
buffer.

buf.slice().retain().release(); // recycled
buf.slice().retain(); buf.release(); // not recycled
Correct range checks in SlicedByteBuf

trustin · 2016-04-13T06:05:35Z

@ninja-, @merlimat, @normanmaurer PTAL. Please let me know if there's any regression. It changes how reference counting works for derived buffers.

I think we can remove the methods that ends with Retained before cherry-picking.

normanmaurer · 2016-04-13T06:48:28Z

buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012 The Netty Project
+ * Copyright 2016 The Netty Project


actually I think we never change the year on existing files. So revert ?

normanmaurer · 2016-04-13T06:57:48Z

buffer/src/main/java/io/netty/buffer/package-info.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012 The Netty Project
+ * Copyright 2016 The Netty Project


Related: #4333 #4421 Motivation: slice(), duplicate() and readSlice() currently create a non-recyclable derived buffer instance. Under heavy load, an application that creates a lot of derived buffers can put the garbage collector under pressure. Modifications: - Deprecate the old derived buffer implementations - Add the new recyclable derived buffer implementations, which has its own reference count value - When a derived buffer is created, its internal reference count is 0. - When retain() is called on a derived buffer, the internal reference count becomes a positive value and the original buffer's retain() is called. - When release() is called on a derived buffer, the internal reference count is decreased first, and then the original buffer's release() is called when the internal reference count is 0. - Add ByteBufUtil.duplicate/slice() so that a user can easily implement ByteBuf.duplicate/slice() - Add missing overrides in some ByteBuf impls - Fix incorrect range checks in SlicedByteBuf - Miscellaneous: - Merge Duplicated/SlicedAbstractByteBuf.unwrap0() into unwrap() using covariant return type Result: - Derived buffers are now recycled when retained and released, although they are not recycled if a user called release() against the original buffer. buf.slice().retain().release(); // recycled buf.slice().retain(); buf.release(); // not recycled - Correct range checks in SlicedByteBuf

normanmaurer · 2016-04-13T09:56:48Z

Talked to @trustin, please hold back with review this needs some changes.

netkins · 2016-04-13T10:44:03Z

buffer/src/main/java/io/netty/buffer/AbstractPooledDerivedByteBuf.java

+    @SuppressWarnings("unchecked")
+    AbstractPooledDerivedByteBuf(Handle<? extends AbstractPooledDerivedByteBuf<T>> recyclerHandle) {
+        super(0);
+        this.recyclerHandle = (Handle<AbstractPooledDerivedByteBuf<T>>) recyclerHandle;


Remove this unnecessary cast to "Handle".

trustin · 2016-04-13T12:42:59Z

Will open a clean PR after rebasing.

Scottmitch · 2016-04-14T00:00:41Z

@trustin - don't forget to delete this branch when its no longer needed.

Related: #4333 #4421 #5128 Motivation: slice(), duplicate() and readSlice() currently create a non-recyclable derived buffer instance. Under heavy load, an application that creates a lot of derived buffers can put the garbage collector under pressure. Modifications: - Add the new recyclable derived buffer implementations, which has its own reference count value - Add ByteBufUtil.duplicateRetained/sliceRetained() so that a user can easily implement ByteBuf.duplicateRetained/sliceRetained() - Use duplicateRetained() and sliceRetained() wherever possible Result: Derived buffers are now recycled when created via sliceRetained() and duplicateRetained()

Related: #4333 #4421 #5128 Motivation: slice(), duplicate() and readSlice() currently create a non-recyclable derived buffer instance. Under heavy load, an application that creates a lot of derived buffers can put the garbage collector under pressure. Modifications: - Add ByteBuf.rslice/rduplicate/readRSlice() which creates a non-recyclable derived buffer - Add the new recyclable derived buffer implementations, which has its own reference count value - Add ByteBufUtil.rduplicate/rslice() so that a user can easily implement ByteBuf.rduplicate/rslice() - Add ByteBufHolder.rduplicate() - Add a new protected method DefaultByteBufHolder.copy(ByteBuf) so that a user can implement copy/duplicate/rduplicate() easily - Use rduplicate() and rslice() wherever possible Result: Derived buffers are now recycled when created via rslice() and rduplicate()

Related: #4333 #4421 #5128 Motivation: slice(), duplicate() and readSlice() currently create a non-recyclable derived buffer instance. Under heavy load, an application that creates a lot of derived buffers can put the garbage collector under pressure. Modifications: - Add the following methods which creates a non-recyclable derived buffer - retainedSlice() - retainedDuplicate() - readRetainedSlice() - Add the new recyclable derived buffer implementations, which has its own reference count value - Add ByteBufUtil.retainedSlice() and retainedDuplicate() so that a user can implement the new ByteBuf methods easily - Add ByteBufHolder.retainedDuplicate() - Add ByteBufHolder.replace(ByteBuf) so that.. - a user can replace the content of the holder in a consistent way - copy/duplicate/retainedDuplicate() can delegate the holder construction to replace(ByteBuf) - Use retainedDuplicate() and retainedSlice() wherever possible - Miscellaneous: - Rename DuplicateByteBufTest to DuplicatedByteBufTest (missing 'D') Result: Derived buffers are now recycled when created via retainedSlice() and retainedDuplicate()

Related: #4333 #4421 #5128 Motivation: slice(), duplicate() and readSlice() currently create a non-recyclable derived buffer instance. Under heavy load, an application that creates a lot of derived buffers can put the garbage collector under pressure. Modifications: - Add the following methods which creates a non-recyclable derived buffer - retainedSlice() - retainedDuplicate() - readRetainedSlice() - Add the new recyclable derived buffer implementations, which has its own reference count value - Add ByteBufUtil.retainedSlice() and retainedDuplicate() so that a user can implement the new ByteBuf methods easily - Add ByteBufHolder.retainedDuplicate() - Add ByteBufHolder.replace(ByteBuf) so that.. - a user can replace the content of the holder in a consistent way - copy/duplicate/retainedDuplicate() can delegate the holder construction to replace(ByteBuf) - Use retainedDuplicate() and retainedSlice() wherever possible - Miscellaneous: - Rename DuplicateByteBufTest to DuplicatedByteBufTest (missing 'D') Result: Derived buffers are now recycled when created via retainedSlice() and retainedDuplicate() and derived from a pooled buffer

Related: #4333 #4421 #5128 Motivation: slice(), duplicate() and readSlice() currently create a non-recyclable derived buffer instance. Under heavy load, an application that creates a lot of derived buffers can put the garbage collector under pressure. Modifications: - Add the following methods which creates a non-recyclable derived buffer - retainedSlice() - retainedDuplicate() - readRetainedSlice() - Add the new recyclable derived buffer implementations, which has its own reference count value - Add ByteBufHolder.retainedDuplicate() - Add ByteBufHolder.replace(ByteBuf) so that.. - a user can replace the content of the holder in a consistent way - copy/duplicate/retainedDuplicate() can delegate the holder construction to replace(ByteBuf) - Use retainedDuplicate() and retainedSlice() wherever possible - Miscellaneous: - Rename DuplicateByteBufTest to DuplicatedByteBufTest (missing 'D') Result: Derived buffers are now recycled when created via retainedSlice() and retainedDuplicate() and derived from a pooled buffer

Related: #4333 #4421 #5128 Motivation: slice(), duplicate() and readSlice() currently create a non-recyclable derived buffer instance. Under heavy load, an application that creates a lot of derived buffers can put the garbage collector under pressure. Modifications: - Add the following methods which creates a non-recyclable derived buffer - retainedSlice() - retainedDuplicate() - readRetainedSlice() - Add the new recyclable derived buffer implementations, which has its own reference count value - Add ByteBufHolder.retainedDuplicate() - Add ByteBufHolder.replace(ByteBuf) so that.. - a user can replace the content of the holder in a consistent way - copy/duplicate/retainedDuplicate() can delegate the holder construction to replace(ByteBuf) - Use retainedDuplicate() and retainedSlice() wherever possible - Miscellaneous: - Rename DuplicateByteBufTest to DuplicatedByteBufTest (missing 'D') - Make ReplayingDecoderByteBuf.reject() return an exception instead of throwing it so that its callers don't need to add dummy return statement Result: Derived buffers are now recycled when created via retainedSlice() and retainedDuplicate() and derived from a pooled buffer

normanmaurer · 2016-05-17T09:17:48Z

Fixed by #5144

trustin assigned normanmaurer Apr 13, 2016

trustin added the feature label Apr 13, 2016

trustin added this to the 4.1.0.Final milestone Apr 13, 2016

normanmaurer reviewed Apr 13, 2016
View reviewed changes

trustin force-pushed the recycle_derived_buf branch from 46e3ce7 to 35c493d Compare April 13, 2016 08:59

Add ByteBuf.isReadOnly()

d0f1f45

netkins reviewed Apr 13, 2016
View reviewed changes

trustin closed this Apr 13, 2016

trustin removed this from the 4.1.0.Final milestone Apr 13, 2016

trustin mentioned this pull request Apr 14, 2016

Make retained derived buffers recyclable #5144

Closed

normanmaurer deleted the recycle_derived_buf branch May 17, 2016 09:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make derived buffers recyclable / Add missing overrides to ByteBuf impls #5128

Make derived buffers recyclable / Add missing overrides to ByteBuf impls #5128

trustin commented Apr 13, 2016

trustin commented Apr 13, 2016

normanmaurer Apr 13, 2016

normanmaurer Apr 13, 2016

normanmaurer commented Apr 13, 2016

netkins Apr 13, 2016

trustin commented Apr 13, 2016

Scottmitch commented Apr 14, 2016

normanmaurer commented May 17, 2016

Make derived buffers recyclable / Add missing overrides to ByteBuf impls #5128

Make derived buffers recyclable / Add missing overrides to ByteBuf impls #5128

Conversation

trustin commented Apr 13, 2016

trustin commented Apr 13, 2016

normanmaurer Apr 13, 2016

Choose a reason for hiding this comment

normanmaurer Apr 13, 2016

Choose a reason for hiding this comment

normanmaurer commented Apr 13, 2016

netkins Apr 13, 2016

Choose a reason for hiding this comment

trustin commented Apr 13, 2016

Scottmitch commented Apr 14, 2016

normanmaurer commented May 17, 2016