Commit Graph

571 Commits

Author SHA1 Message Date
Jakob Buchgraber
6fd0a0c55f Faster and more memory efficient headers for HTTP, HTTP/2, STOMP and SPYD. Fixes #3600
Motivation:

We noticed that the headers implementation in Netty for HTTP/2 uses quite a lot of memory
and that also at least the performance of randomly accessing a header is quite poor. The main
concern however was memory usage, as profiling has shown that a DefaultHttp2Headers
not only use a lot of memory it also wastes a lot due to the underlying hashmaps having
to be resized potentially several times as new headers are being inserted.

This is tracked as issue #3600.

Modifications:
We redesigned the DefaultHeaders to simply take a Map object in its constructor and
reimplemented the class using only the Map primitives. That way the implementation
is very concise and hopefully easy to understand and it allows each concrete headers
implementation to provide its own map or to even use a different headers implementation
for processing requests and writing responses i.e. incoming headers need to provide
fast random access while outgoing headers need fast insertion and fast iteration. The
new implementation can support this with hardly any code changes. It also comes
with the advantage that if the Netty project decides to add a third party collections library
as a dependency, one can simply plug in one of those very fast and memory efficient map
implementations and get faster and smaller headers for free.

For now, we are using the JDK's TreeMap for HTTP and HTTP/2 default headers.

Result:

- Significantly fewer lines of code in the implementation. While the total commit is still
  roughly 400 lines less, the actual implementation is a lot less. I just added some more
  tests and microbenchmarks.

- Overall performance is up. The current implementation should be significantly faster
  for insertion and retrieval. However, it is slower when it comes to iteration. There is simply
  no way a TreeMap can have the same iteration performance as a linked list (as used in the
  current headers implementation). That's totally fine though, because when looking at the
  benchmark results @ejona86 pointed out that the performance of the headers is completely
  dominated by insertion, that is insertion is so significantly faster in the new implementation
  that it does make up for several times the iteration speed. You can't iterate what you haven't
  inserted. I am demonstrating that in this spreadsheet [1]. (Actually, iteration performance is
  only down for HTTP, it's significantly improved for HTTP/2).

- Memory is down. The implementation with TreeMap uses on avg ~30% less memory. It also does not
  produce any garbage while being resized. In load tests for GRPC we have seen a memory reduction
  of up to 1.2KB per RPC. I summarized the memory improvements in this spreadsheet [1]. The data
  was generated by [2] using JOL.

- While it was my original intend to only improve the memory usage for HTTP/2, it should be similarly
  improved for HTTP, SPDY and STOMP as they all share a common implementation.

[1] https://docs.google.com/spreadsheets/d/1ck3RQklyzEcCLlyJoqDXPCWRGVUuS-ArZf0etSXLVDQ/edit#gid=0
[2] https://gist.github.com/buchgr/4458a8bdb51dd58c82b4
2015-08-04 17:12:24 -07:00
tczerwinski
b33c7b12a4 XmlFrameDecoder is corrupt
Motivation:

Two problems:
1. Decoder assumption that as soon as it finds </ element it can decrement opened xml brackets counter. It can lead to bugs when closing bracket is not in byteBuf yet.
2. Not proper handling of more than two root elements in XML document. First element will be processed properly, second one not. It is caused by assumption that byteBuf readerIndex is 0 at the begging of decoding.

Modifications:

Both problems were resolved by fixes:
1. decrement opened brackets count only if </ > enclosing bracket is found
2. consider readerIndex higher than 0 when counting output frame length

Result:

Both problems were resolved
2015-07-29 18:49:26 +02:00
Norman Maurer
c53dbb748e Add ProtocolDetectionResult and use it in HAProxyMessageDecoder for allow detect HAProxy protocol.
Motivation:

Sometimes it is useful to detect if a ByteBuf contains a HAProxy header, for example if you want to write something like the PortUnification example.

Modifications:

- Add ProtocolDetectionResult which can be used as a return type for detecting different protocol.
- Add new method which allows to detect HA Proxy messages.

Result:

Easier to detect protocol.
2015-06-23 08:59:07 +02:00
Trustin Lee
950da2eae1 Allow MessageAggregator to disallow non-empty content
Motivation:

A user sometimes just want the aggregated message has no content at
all. (e.g. A user only wants HTTP GET requests.)

Modifications:

- Do not raise IllegalArgumentException even if a user specified
  the maxContentLength of 0

Result:

A user can disallow a message with non-empty content.
2015-06-10 12:06:27 +09:00
Norman Maurer
f23b7b4efd [maven-release-plugin] prepare for next development iteration 2015-05-07 14:21:08 -04:00
Norman Maurer
871ce43b1f [maven-release-plugin] prepare release netty-4.1.0.Beta5 2015-05-07 14:20:38 -04:00
Norman Maurer
891be30a28 Remove memory copy when extract frame in LengthFieldBasedFrameDecoder
Motivation:

We are currently doing a memory cop to extract the frame in LengthFieldBasedFrameDecoder which can be eliminated.

Modifications:

Use buffer.slice(...).retain() to eliminate the memory copy.

Result:

Better performance.
2015-04-29 08:43:06 +02:00
Alwayswithme
abccf18411 fix the discardedBytes counting on LineBasedFrameDecoder
Motivation:

The LineBasedFrameDecoder discardedBytes counting different compare to
DelimiterBasedFrameDecoder.

Modifications:

Add plus sign

Result:

DiscardedBytes counting correctly
2015-04-21 09:25:47 +02:00
Norman Maurer
62057f73d6 Fix handling of non-auto read for ByteToMessageDecoder and SslHandler
Motivation:

Our automatically handling of non-auto-read failed because it not detected the need of calling read again by itself if nothing was decoded. Beside this handling of non-auto-read never worked for SslHandler as it always triggered a read even if it decoded a message and auto-read was false.

This fixes [#3529] and [#3587].

Modifications:

- Implement handling of calling read when nothing was decoded (with non-auto-read) to ByteToMessageDecoder again
- Correctly respect non-auto-read by SslHandler

Result:

No more stales and correctly respecting of non-auto-read by SslHandler.
2015-04-20 09:11:02 +02:00
Norman Maurer
ccde870b38 Revert "Ensure channelReadComplete() is called only when necessary"
This reverts commit 27a25e29f7.
2015-04-20 09:10:41 +02:00
Norman Maurer
bb692816d2 Revert "Do not suppress channelReadComplete() when a handler was just added"
This reverts commit 720faa4df1.
2015-04-20 09:10:29 +02:00
Scott Mitchell
9a7a85dbe5 ByteString introduced as AsciiString super class
Motivation:
The usage and code within AsciiString has exceeded the original design scope for this class. Its usage as a binary string is confusing and on the verge of violating interface assumptions in some spots.

Modifications:
- ByteString will be created as a base class to AsciiString. All of the generic byte handling processing will live in ByteString and all the special character encoding will live in AsciiString.

Results:
The AsciiString interface will be clarified. Users of AsciiString can now be clear of the limitations the class imposes while users of the ByteString class don't have to live with those limitations.
2015-04-14 16:35:17 -07:00
Norman Maurer
aa1e537de4 [#3373] Rename class to match naming scheme
Motivation:

The ReplayingDecoderBuffer does not match the naming scheme we use for ByteBuf types.

Modifications:

Rename to ReplayingDecoderByteBuf to match naming scheme

Result:

Consistent naming
2015-04-12 13:32:41 +02:00
Idel Pivnitskiy
c9adb41636 Refactor tests for compression codecs
Motivation:

Too many duplicated code of tests for different compression codecs.

Modifications:

- Added abstract classes AbstractCompressionTest, AbstractDecoderTest and AbstractEncoderTest which contains common variables and tests for any compression codec.
- Removed common tests which are implemented in AbstractDecoderTest and AbstractEncoderTest from current tests for compression codecs.
- Implemented abstract methods of AbstractDecoderTest and AbstractEncoderTest in current tests for compression codecs.
- Added additional checks for current tests.
- Renamed abstract class IntegrationTest to AbstractIntegrationTest.
- Used Theories to run tests with head and direct buffers.
- Removed code duplicates.

Result:

Removed duplicated code of tests for compression codecs and simplified an addition of tests for new compression codecs.
2015-04-10 15:50:41 +02:00
Robert.Panzer
18443efeab Add support for byte order to LengthFieldPrepender
Motivation:

While the LengthFieldBasedFrameDecoder supports a byte order the LengthFieldPrepender does not.
That means that I can simply add a LengthFieldBasedFrameDecoder with ByteOrder.LITTLE_ENDIAN to my pipeline
but have to write my own Encoder to write length fields in little endian byte order.

Modifications:

Added a constructor that takes a byte order and all other parameters.
All other constructors delegate to this one with ByteOrder.BIG_ENDIAN.
LengthFieldPrepender.encode() uses this byte order to write the length field.

Result:

LengthFieldPrepender will write the length field in the defined byte order.
2015-03-13 18:50:20 +01:00
Leo Gomes
01f80d96b6 Add unit to maxContentLength message javadoc
Motivation:
Not knowing which unit is returned by the maxContentLength() of the Messageggregator when reading the Javadoc is annoying and can be a source of bugs.

Modifications:
Added the mention "in bytes"

Result:
Javadoc is clear.
2015-03-05 20:54:01 +01:00
Norman Maurer
fce0989844 [maven-release-plugin] prepare for next development iteration 2015-03-03 02:06:47 -05:00
Norman Maurer
ca3b1bc4b7 [maven-release-plugin] prepare release netty-4.1.0.Beta4 2015-03-03 02:05:52 -05:00
Daniel Bevenius
c53b8d5a85 Suggestion for supporting single header fields.
Motivation:
At the moment if you want to return a HTTP header containing multiple
values you have to set/add that header once with the values wanted. If
you used set/add with an array/iterable multiple HTTP header fields will
be returned in the response.

Note, that this is indeed a suggestion and additional work and tests
should be added. This is mainly to bring up a discussion.

Modifications:
Added a flag to specify that when multiple values exist for a single
HTTP header then add them as a comma separated string.
In addition added a method to StringUtil to help escape comma separated
value charsequences.

Result:
Allows for responses to be smaller.
2015-02-18 10:54:15 +01:00
Cristian
b8d26d1da3 Avoid unnecessary call to ByteBuf.isReadable() from ByteToMessageDecoder
Motivation:

This will avoid one unncessary method invokation which will slightly improve performance.

Modifications:

Instead of calling isReadable we just check for the value of readableBytes()

Result:

Nothing functionally speaking change.
2015-02-16 07:37:20 +01:00
Trustin Lee
976db9269d Revamp io.netty.handler.codec.socksx
While implementing netty-handler-proxy, I realized various issues in our
current socksx package. Here's the list of the modifications and their
background:

- Split message types into interfaces and default implementations
  - so that a user can implement an alternative message implementations
- Use classes instead of enums when a user might want to define a new
  constant
  - so that a user can extend SOCKS5 protocol, such as:
    - defining a new error code
    - defining a new address type
- Rename the message classes
  - to avoid abbreviated class names. e.g:
    - Cmd -> Command
    - Init -> Initial
  - so that the class names align better with the protocol
    specifications. e.g:
    - AuthRequest -> PasswordAuthRequest
    - AuthScheme -> AuthMethod
- Rename the property names of the messages
  - so that the property names align better when the field names in the
    protocol specifications
- Improve the decoder implementations
  - Give a user more control over when a decoder has to be removed
  - Use DecoderResult and DecoderResultProvider to handle decode failure
    gracefully. i.e. no more Unknown* message classes
- Add SocksPortUnifinicationServerHandler since it's useful to the users
  who write a SOCKS server
  - Cleaned up and moved from the socksproxy example
2015-02-10 09:14:13 +09:00
Brendt Lucas
aa8917115c Determine CompositeByteBuf implementation by using ByteBufAllocator
Motivation:

Currently, using a MessageAggregator in the pipeline always results in the creation of an unpooled heap CompositeByteBuf. By using the ByteBufAllocator the CompositeByteBuf will use the implementation specified by the ByteBufAllocator.

Modifications:

Use the ChannelHandlerContext's ByteBufAllocator to create the CompositeByteBuf for message aggregation

Result:

The CompositeByteBuf is now configured based on the ByteBufAllocator's settings.
2015-02-08 15:46:06 +01:00
Trustin Lee
720faa4df1 Do not suppress channelReadComplete() when a handler was just added
Related:
- 27a25e29f7

Motivation:

The commit mentioned above introduced a regression where
channelReadComplete() event is swallowed by a handler which was added
dynamically.

Modifications:

Do not suppress channelReadComplete() if the current handler's
channelRead() method was not invoked at all, so that a just-added
handler does not suppress channelReadComplete().

Result:

Regression is gone, and channelReadComplete() is invoked when necessary.
2015-02-07 23:09:07 +09:00
Trustin Lee
27a25e29f7 Ensure channelReadComplete() is called only when necessary
Motivation:

Even if a handler called ctx.fireChannelReadComplete(), the next handler
should not get its channelReadComplete() invoked if fireChannelRead()
was not invoked before.

Modifications:

- Ensure channelReadComplete() is invoked only when the handler of the
  current context actually produced a message, because otherwise there's
  no point of triggering channelReadComplete().
  i.e. channelReadComplete() must follow channelRead().
- Fix a bug where ctx.read() was not called if the handler of the
  current context did not produce any message, making the connection
  stall. Read the new comment for more information.

Result:

- channelReadComplete() is invoked only when it makes sense.
- No stale connection
2015-02-07 16:13:56 +09:00
Marco Craveiro
6d07264412 Minor idiomatic changes to java docs 2015-02-04 08:28:29 +01:00
Scott Mitchell
a9577c0a4b Zlib decoder calls reduction and index fix
Motivation:
The JdkZlibDecoder and JZlibDecoder call isReadable and readableBytes in the same method. There is an opportunity to reduce the number of methods calls to just use readableBytes.  JdkZlibDecoder reads from a ByteBuf with an absolute index instead of using readerIndex()

Modifications:
- Use readableBytes where isReadable was used
- Correct absolute ByteBuf index to be relative to readerIndex()

Result:
Less method calls duplicating work and preventing an index out of bounds exception.
2015-01-26 20:48:27 +01:00
Scott Mitchell
c4a5c3966c Headers remove infrequently used member variables
Motivation:
There are two member variables (addAllVisitor, setAllVisitor) which are likely not to be used in the majority of use cases.

Modifications:
Remove these member variables and rely on a method to return a new object when needed.

Result:
Two less member variables for each DefaultHeaders instance.
2015-01-22 15:43:18 -05:00
Scott Mitchell
27a2017f7f Opportunity for lazy initialization in Headers interface
Motivation:
The Headers interface had two member variables (addAllVisitor, setAllVisitor) which are not necessarily always needed but are always instantiated.  This may result in excess memory being used.

Modifications:
 - addAllVisitor will be accessed via a method addAllVisitor() which will use lazy initialization.
 - setAllVisitor will be accessed via a method addAllVisitor() which will use lazy initialization.

Result:
Potential memory savings by using lazy initialization.
2015-01-21 13:48:52 -05:00
Trustin Lee
3ebe2ee369 Remove unnecessary loop and indentation in decompressors
Motivation:

Decompression handlers contain heavy use of switch-case statements. We
use compact indentation style for 'case' so that we utilize our screen
real-estate more efficiently.

Also, the following decompression handlers do not need to run a loop,
because ByteToMessageDecoder already runs a loop for them:

- FastLzFrameDecoder
- Lz4FrameDecoder
- LzfDecoder

Modifications:

- Fix indentations
- Do not wrap the decoding logic with a for loop when unnecessary
- Handle the case where a FastLz/Lzf frame contains no data properly so
  that the buffer does not leak and less garbage is produced.

Result:

- Efficiency
- Compact source code
- No buffer leak
2015-01-12 00:18:38 +09:00
Norman Maurer
50af9b916c Eliminate memory copy in ByteToMessageDecoder whenever possible
Motivation:

Currently when there are bytes left in the cumulation buffer we do a byte copy to produce the input buffer for the decode method. This can put quite some overhead on the impl.

Modification:

- Use a CompositeByteBuf to eliminate the byte copy.
- Allow to specify if a CompositeBytebug should be used or not as some handlers can only act on one ByteBuffer in an efficient way (like SslHandler :( ).

Result:

Performance improvement as shown in the following benchmark.

Without this patch:
[xxx@xxx ~]$ ./wrk-benchmark
Running 5m test @ http://xxx:8080/plaintext
  16 threads and 256 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    20.19ms   38.34ms   1.02s    98.70%
    Req/Sec   241.10k    26.50k  303.45k    93.46%
  1153994119 requests in 5.00m, 155.84GB read
Requests/sec: 3846702.44
Transfer/sec:    531.93MB

With the patch:
[xxx@xxx ~]$ ./wrk-benchmark
Running 5m test @ http://xxx:8080/plaintext
  16 threads and 256 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    17.34ms   27.14ms 877.62ms   98.26%
    Req/Sec   252.55k    23.77k  329.50k    87.71%
  1209772221 requests in 5.00m, 163.37GB read
Requests/sec: 4032584.22
Transfer/sec:    557.64MB
2015-01-09 15:56:30 +09:00
Leonardo Freitas Gomes
9ee75126eb Motivation: Sonar points out an equals comparison, where the types compared are different and don't share any common parent http://clinker.netty.io/sonar/drilldown/issues/io.netty:netty-parent:master?severity=CRITICAL#
Modifications:
Converted AsciiString into a String by calling toString() method before comparing with equals(). Also added a unit-test to show that it works.

Result:
Major violation is gone. Code is correct.
2014-12-16 07:17:31 +01:00
Scott Mitchell
8206cc6e14 Headers set/add/contains timeMillis methods
Motivation:
The new Headers interface contains methods to getTimeMillis but no add/set/contains variants.  These should be added for consistency.

Modifications:
- Add three new methods: addTimeMillis, setTimeMillis, containsTimeMillis to the Headers interface.
- Add a new method to the Headers.ValueConverter interface: T convertTimeMillis(long)
- Bring these new interfaces up the class hierarchy

Result:
All Headers classes have setters/getters for timeMillis.
2014-12-06 22:40:28 +09:00
Ronald Chen
bbe880f5ea Rocumented decoder pitfalls to avoid mistakes found in [#3184] 2014-12-01 20:25:56 +01:00
Idel Pivnitskiy
35db3c6710 Small performance improvements
Motivation:

Found performance issues via FindBugs and PMD.

Modifications:

- Removed unnecessary boxing/unboxing operations in DefaultTextHeaders.convertToInt(CharSequence) and DefaultTextHeaders.convertToLong(CharSequence). A boxed primitive is created from a string, just to extract the unboxed primitive value.
- Added a static modifier for DefaultHttp2Connection.ParentChangedEvent class. This class is an inner class, but does not use its embedded reference to the object which created it. This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary.
- Added a static compiled Pattern to avoid compile it each time it is used when we need to replace some part of authority.
- Improved using of StringBuilders.

Result:

Performance improvements.
2014-11-20 00:10:06 -05:00
Trustin Lee
1765429335 Revert bad renaming in ZlibTest 2014-11-19 18:36:23 +09:00
Trustin Lee
0795ee6130 Add more test cases to ZlibTest
Motivation:

Currently, we only test our ZlibEncoders against our ZlibDecoders. It is
convenient to write such tests, but it does not necessarily guarantee
their correctness. For example, both encoder and decoder might be faulty
even if the tests pass.

Modifications:

Add another test that makes sure that our GZIP encoder generates the
GZIP trailer, using the fact that GZIPInputStream raises an EOFException
when GZIP trailer is missing.

Result:

More coverage for GZIP compression
2014-11-19 18:15:56 +09:00
Scott Mitchell
72a611a28f HTTP Content Encoder allow EmptyLastHttpContent
Motiviation:
The HttpContentEncoder does not account for a EmptyLastHttpContent being provided as input.  This is useful in situations where the client is unable to determine if the current content chunk is the last content chunk (i.e. a proxy forwarding content when transfer encoding is chunked).

Modifications:
- HttpContentEncoder should not attempt to compress empty HttpContent objects

Result:
HttpContentEncoder supports a EmptyLastHttpContent to terminate the response.
2014-11-05 23:31:27 -05:00
Trustin Lee
53fbfbb590 Remove CollectionUtils
Motivation:

CollectionUtils has only one method and it is used only in DefaultHeaders.

Modification:

Move CollectionUtils.equals() to DefaultHeaders and make it private

Result:

One less class to expose in our public API
2014-11-01 02:59:47 +09:00
Scott Mitchell
50e06442c3 Backport header improvements from 5.0
Motivation:
The header class hierarchy and algorithm was improved on the master branch for versions 5.x. These improvments should be backported to the 4.1 baseline.

Modifications:
- cherry-pick the following commits from the master branch: 2374e17, 36b4157, 222d258

Result:
Header improvements in master branch are available in 4.1 branch.
2014-11-01 00:59:57 +09:00
Idel Pivnitskiy
5f94d7a319 Refactor LzfDecoder to use proper state machine
Motivation:

Make it much more readable code.

Modifications:

- Added states of decompression.
- Refactored decode(...) method to use this states.

Result:

Much more readable decoder which looks like other compression decoders.
2014-10-20 13:59:54 +02:00
Idel Pivnitskiy
8988b3f4c0 Fixed typo in LzmaFrameEncoder constructor
Motivation:

A discovered typo in LzmaFrameEncoder constructor when we check `lc + lp` for better compatibility.

Modifications:

Changed `lc + pb` to `lc + lp`.

Result:

Correct check of `lc + lp` value.
2014-09-19 20:09:39 +02:00
Idel Pivnitskiy
cf5aea52ed Implemented LZMA frame encoder
Motivation:

LZMA compression algorithm has a very good compression ratio.

Modifications:

- Added `lzma-java` library which implements LZMA algorithm.
- Implemented LzmaFrameEncoder which extends MessageToByteEncoder and provides compression of outgoing messages.
- Added tests to verify the LzmaFrameEncoder and how it can compress data for the next uncompression using the original library.

Result:

LZMA encoder which can compress data using LZMA algorithm.
2014-09-15 15:05:36 +02:00
Barber, Francis
08cec3c56b Add support for ExtensionRegistryLite in ProtobufDecoder
Motivation:

ExtensionRegistry is a subclass of ExtensionRegistryLite.  The ProtobufDecoder
doesn't use the registry directly, it simply passes it through to the Protobuf
API.  The Protobuf calls in question are themselves written in terms
ExtensionRegistryLite not ExtensionRegistry.

Modifications:

Require ExtensionRegistryLite instead of ExtensionRegistry in ProtobufDecoder.

Result:

Consumers can use ExtensionRegistryLite with ProtobufDecoder.
2014-09-15 07:06:04 +02:00
Trustin Lee
2026f15be2 Check noJdkZlibEncoder before comparing other parameters
.. for consistency
2014-08-26 16:12:31 +09:00
Trustin Lee
8cb6b779c9 Add io.netty.noJdkZlibEncoder system property
Related issue: #2821

Motivation:

There's no way for a user to change the default ZlibEncoder
implementation.

It is already possible to change the default ZlibDecoder implementation.

Modification:

Add a new system property 'io.netty.noJdkZlibEncoder'.

Result:

A user can disable JDK ZlibEncoder, just like he or she can disable JDK
ZlibDecoder.
2014-08-26 15:51:11 +09:00
Norman Maurer
b201877734 Move duplicated code into CodecUtil
Motivation:

We have some duplicated code that can be reused.

Modifications:

Create package private class called CodecUtil that now contains the shared code / helper method.

Result:

Less code-duplication
2014-08-25 07:37:23 +02:00
Norman Maurer
fbf8533759 [#2812] Ensure we call checkForSharableAnnotation in all constructors of ByteToMessageCodec
Motivation:

ByteToMessageCodec miss to check for @Sharable annotation in one of its constructors.

Modifications:

Ensure we call checkForSharableAnnotation in all constructors.

Result:

After your change, what will change.
2014-08-23 21:03:25 +02:00
Trustin Lee
b5f61d0de5 [maven-release-plugin] prepare for next development iteration 2014-08-16 03:27:42 +09:00
Trustin Lee
76ac3b21a5 [maven-release-plugin] prepare release netty-4.1.0.Beta3 2014-08-16 03:27:37 +09:00
Trustin Lee
b3c1904cc9 [maven-release-plugin] prepare for next development iteration 2014-08-15 09:31:03 +09:00