Commit Graph

6989 Commits

Author SHA1 Message Date
Scott Mitchell
6cb6282699 Lazy Initialization of epoll splice queue
Motivation:
AbstractEpollStreamChannel has a queue which collects splice events. Splice is assumed not to be the most common use case of this class and thus the splice queue could be initialized in a lazy fashion to save memory. This becomes more significant when the number of connections grows.

Modifications:
- AbstractEpollStreamChannel.spliceQueue will be initialized in a lazy fashion

Result:
Less memory consumption for most use cases
2015-11-20 15:09:53 -08:00
Scott Mitchell
49cd00da1c Backport of benchmark broke build
Motivation:
2a2059d976 was backported from master, and included an overriden method which does not exist in 4.1.

Modifications:
- Remove the invoker method from NoPriorityByteDistributionBenchmark

Result:
No more build error
2015-11-20 15:03:50 -08:00
Norman Maurer
7bee318fc7 Use OneTimeTask where possible to reduce object creation
Motivation:

We should use OneTimeTask where possible to reduce object creation.

Modifications:

Replace Runnable with OneTimeTask

Result:

Less object creation
2015-11-20 14:39:06 -08:00
Scott Mitchell
1f3fc983c0 DefaultPromise LateListener notification order
Motivation:
There is a notification ordering issue in DefaultPromise when the lateListener collection is in use. The ordering issue can be observed in situations where a late listener is added to a Future returned from a write operation. It is possible that this future will run after a read operation scheduled on the I/O thread, even if the late listener is added on the I/O thread. This can lead to unexpected ordering where a listener for a write operation which must complete in order for the read operation to happen is notified after the read operation is done.

Modifications:
- If the lateListener collection becomes empty, it should be treated as though it was null when checking if lateListeners can be notified immediatley (instead of executing a task on the executor)

Result:
Ordering is more natural and will not be perceived as being out of order relative to other tasks on the same executor.
2015-11-20 09:30:35 -08:00
Norman Maurer
cfa76f6326 Remove unnecessary reference to AbstractChannel from AbstractChannelHandlerContext
Motivation:

We not need to store another reference to AbstractChannel as we can access it through DefaultChannelHandlerContext.

Modifications:

Remove reference.

Result:

Cleaner code.
2015-11-20 06:41:11 -08:00
Norman Maurer
2d2e07578a Remove HashMap for lookup name / ctx from DefaultChannelPipeline to reduce memory footprint
Motivation:

If you start to have 1M+ concurrent connections memory footprint can be come a big issue. We should try to reduce it as much as possible in the core of netty.

Modifications:

- Remove HashMap that was used to store name to ctx mapping. This was only used for validation and access a handler by name. As a pipeline is not expected to be very long (like 100+ handlers) we can just walk the linked list structure to find the ctx with a given name.

Result:

Less memory footprint of the DefaultChannelPipeline.
2015-11-20 06:29:28 -08:00
Norman Maurer
edb2250d35 Store reference to IovArray in the EpollEventLoop to reduce thread local access.
Motivation:

If we have a lot of writes going on we currently need to lookup the IovArray for each Channel that does writes. This can have quite some perf overhead. We should not need to do this and just store a reference of the IovArray on the EpollEventLoop itself.

Modifications:

- Remove IoArrayThreadLocal
- Store the IoArray in the EventLoop itself

Result:

Less FastThreadLocal lookups
2015-11-20 06:07:54 -08:00
nmittler
2a2059d976 Adding UniformStreamByteDistributor
Motivation:

The current priority algorithm can yield poor per-stream goodput when either the number of streams is high or the connection window is small. When all priorities are the same (i.e. priority is disabled), we should be able to do better.

Modifications:

Added a new UniformStreamByteDistributor that ignores priority entirely and manages a queue of streams.  Each stream is allocated a minimum of 1KiB on each iteration.

Result:

Improved goodput when priority is not used.
2015-11-19 16:49:12 -08:00
Scott Mitchell
b640de2d94 Epoll Shutdown Input Exception Handling
Motivation:
If ChannelOption.ALLOW_HALF_CLOSURE is true and the shutdown input operation fails we should not propagate this exception, and instead consider this socket's read as half closed.

Modifications:
- AbstractEpollChannel.shutdownInput should not propagate exceptions when attempting to shutdown the input, but instead should just close the socket

Result:
Users expecting a ChannelInputShutdownEvent will get this event even if the socket is already shutdown, and the shutdown operation fails.
2015-11-19 16:03:10 -08:00
Dmitry Spikhalskiy
2a65ae256e [#4331] Helper methods to get charset from Content-Type header of HttpMessage
Motivation:

HttpHeaders already has specific methods for such popular and simple headers like "Host", but if I need to convert POST raw body to string I need to parse complex ContentType header in my code.

Modifications:

Add getCharset and getCharsetAsString methods to parse charset from Content-Length header.

Result:

Easy to use utility method.
2015-11-19 15:59:34 -08:00
pieteradejong
64409ad10b fixed word - issue #4469 2015-11-19 07:33:02 -08:00
nmittler
96f9b0b91b Remote flow controller incorrectly updates stream state
Motivation:

The `DefaultHttp2RemoteFlowController` does not correctly determine `hasFrame` when updating the stream state for the distributor. Adding a check to enforce `hasFrame` when `streamableBytes > 0` causes several test failures.

Modifications:

Modified `DefaultHttp2RemoteFlowController` to simplify the writing logic and to correct the bookkeeping for `hasFrame`.

Result:

The distributors are always called with valid arguments.
2015-11-18 11:32:18 -08:00
Janecek Jakub
a4ebdd0eca Fix setBytes on read-only ByteBuffer
Motivation:

The method setBytes did not work correctly because read-only ByteBuffer
does not allow access to its underlying array.

Modifications:

New case was added for ByteBuffer's that are not direct and do not have an array.
These must be handled by copying the data into a temporary array. Unit test was
added to test this case.

Result:

It is now possible to use read-only ByteBuffer as the source
for the setBytes method.
2015-11-17 22:45:37 -08:00
nmittler
8accc52b03 Forking Twitter's hpack
Motivation:

The twitter hpack project does not have the support that it used to have.  See discussion here: https://github.com/netty/netty/issues/4403.

Modifications:

Created a new module in Netty and copied the latest from twitter hpack master.

Result:

Netty no longer depends on twitter hpack.
2015-11-14 10:13:32 -08:00
Norman Maurer
2ecce8fa56 [maven-release-plugin] prepare for next development iteration 2015-11-10 22:59:33 +01:00
Norman Maurer
6a93f331d3 [maven-release-plugin] prepare release netty-4.1.0.Beta8 2015-11-10 22:50:57 +01:00
Scott Mitchell
b4b791353d AsciiString optimized hashCode
Motivation:
The AsciiString.hashCode() method can be optimized. This method is frequently used while to build the DefaultHeaders data structure.

Modification:
- Add a PlatformDependent hashCode algorithm which utilizes UNSAFE if available

Result:
AsciiString hashCode is faster.
2015-11-10 10:28:31 -08:00
Scott Mitchell
0d71744d5b IllegalRefCountException in FullHttp[Request|Response].hashCode()
Motivation:
FullHttp[Request|Response].hashCode() uses a releasable object and in vulnerable to a IllegalRefCountException if that object has been released.

Modifications:
- Ensure the released object is not used.

Result:
No more IllegalRefCountException.
2015-11-09 16:47:29 -08:00
Trustin Lee
120ffaf880 Use separate query ID space for different DNS servers
Related: #3972

Motivation:

DnsNameResolver limits the number of concurrent in-progress DNS queries
to 65536 regardless the number of DNS servers it communicates with. When
the number of available DNS servers are more than just one, we end up
using much less (65536 / numDnsServers) query IDs per DNS server, which
is non-optimal.

Modifications:

- Replace the query ID and context management with
  DnsQueryContextManager
  - Eash DNS server gets its own query ID space

Result:

Much bigger query ID space, and thus there's less chance of getting the
'query ID space exhaustion' error
2015-11-09 15:25:13 -08:00
Sylwester Lachiewicz
a18416df60 Export defaults from PooledByteBufAcclocator static fields
Motivation:

Allow external application to tune initialization of PooledByteBufAllocator

Modifications:

Added new static methods

Result:

Exported
DEFAULT_NUM_HEAP_ARENA
DEFAULT_NUM_DIRECT_ARENA
DEFAULT_PAGE_SIZE
DEFAULT_MAX_ORDER
DEFAULT_TINY_CACHE_SIZE
DEFAULT_SMALL_CACHE_SIZE
DEFAULT_NORMAL_CACHE_SIZE
2015-11-08 08:08:23 +01:00
Scott Mitchell
035053be4a HTTP/2 Client Example TLS Pipeline Initialization
Motivation:
The HTTP/2 client example is not validating the results of ALPN if TLS is enabled.

Modifications:
- Use ApplicationProtocolNegotiationHandler to validate ALPN results.

Result:
Client example validates ALPN results.
2015-11-07 10:15:20 -08:00
Scott Mitchell
2f81364522 DefaultHttp2HeadersTest updates
Motivation:
Recently a bug was found in DefaultHttp2Headers where the state of the headers could be corrupted due to the extra tracking to make pseudo headers first during iteration. Unit tests did not catch this bug.

Modifications:
- Update unit tests to cover more methods

Result:
Unit tests for DefaultHttp2Headers have better code coverage.
2015-11-07 10:14:00 -08:00
Scott Mitchell
8f07416baf Update ALPN boot version
Motivation:
A new version of ALPN boot has been released.

Modifications:
- Update the pom to pull in this new version

Result:
New JDK get new ALPN boot.
2015-11-07 09:37:56 -08:00
Scott Mitchell
21e27da410 ApplicationProtocolNegotiationHandler failure behavior
Motivation:
Child classes of ApplicationProtocolNegotiationHandler may want to override the behavior when a handshake failure is detected.

Modifications:
- Provide a method which can be overriden when a handshake failure is detected.

Result:
Child classes can override ApplicationProtocolNegotiationHandler handshake failure behavior.
2015-11-07 09:33:08 -08:00
Louis Ryan
6e108cb96a Improve the performance of copying header sets when hashing and name validation are equivalent.
Motivation:
Headers and groups of headers are frequently copied and the current mechanism is slower than it needs to be.

Modifications:
Skip name validation and hash computation when they are not necessary.
Fix emergent bug in CombinedHttpHeaders identified with better testing
Fix memory leak in DefaultHttp2Headers when clearing
Added benchmarks

Result:
Faster header copying and some collateral bug fixes
2015-11-07 08:53:10 -08:00
Sergio Bossa
187efca9aa Improved DefaultChannelPipeline#destroy() to avoid spinning continuously in case of custom executors.
Motivation:

The previous DefaultChannelPipeline#destroy() implementation, introduced in #3156, is suboptimal as it can cause the for loop to continuously spin if the executor used by a given handler is unable to "recognize" the event loop.
It could be objected that it's the custom executor responsibility to properly implement the inEventLoop() method, but some implementetaions might not be able to do that for performance reasons, and even so, it's always better to be safe against API misuse, in particular when it is not possible to fail fast and the alternative is rather some sutle behaviour.

Modifications:

The patch simply avoids the recursive spin by explicitly passing the "in event loop" condition as a boolean parameter, preserving the same guarantees offered by #3156. A unit test has also been added.

Result:

All channel events are correctly called and no high CPU usage is seen anymore.
2015-11-06 19:37:53 +01:00
Louis Ryan
7cc320ce47 Fix memory leak in DefaultHttp2Headers
Motivation:

Memory leak makes headers non-reusable.

Modifications:

Correctly reset firstNonPseudo header reference

Result:

No leak
2015-11-06 07:08:57 -08:00
Louis Ryan
3eb65797ed Make headers.set(self) a no-op instead of throwing. Makes it consistent with setAll
Motivation:

Makes the API contract of headers more consistent and simpler.

Modifications:

If self is passed to set then simply return

Result:

set and setAll will be consistent
2015-11-06 07:00:54 -08:00
nmittler
6504d52b94 Add HTTP/2 local flow control option for auto refill
Motivation:

For many HTTP/2 applications (such as gRPC) it is necessary to autorefill the connection window in order to prevent application-level deadlocking.

Consider an application with 2 streams, A and B.  A receives a stream of messages and the application pops off one message at a time and makes a request on stream B. However, if receiving of data on A has caused the connection window to collapse, B will not be able to receive any data and the application will deadlock.  The only way (currently) to get around this is 1) use multiple connections, or 2) manually refill the connection window.  Both are undesirable and could needlessly complicate the application code.

Modifications:

Add a configuration option to DefaultHttp2LocalFlowController, allowing it to autorefill the connection window.

Result:

Applications can configure HTTP/2 to avoid inter-stream deadlocking.
2015-11-05 15:47:10 -08:00
Vladimir Krivosheev
a4f3e72e71 configurable service thread name prefix
Motivation:

If netty used as part of application, should be a way to prefix service thread name to easy distinguish such threads (for example, used in IntelliJ Platform)

Modifications:

Introduce system property io.netty.serviceThreadPrefix

Result:

ThreadDeathWatcher thread has a readable name "Netty threadDeathWatcher-2-1" if io.netty.serviceThreadPrefix set to "Netty"
2015-11-05 08:51:12 +01:00
Norman Maurer
98eb69f169 Remove synchronization overhead on generateName.
Motivation:

Changing the chache of generated names to use a cache per thread. This will remove the bottleneck when many eventloops are used and names need to generate.

Modifications:

Use a FastThreadLocal to store the cached names.

Result:

Less locking between threads.
2015-11-04 20:41:41 +01:00
Scott Mitchell
91b8ef3d10 HTTP/2 PriorityStreamByteDistributor exceptions and reentry
Motivation:
PriorityStreamByteDistributor saves exception state and attempts to reset state. This could be simplified by just throwing a connection error and closing the connection. PriorityStreamByteDistributor also does not handle or detect re-entry in the distribute method.

Motivation:
- PriorityStreamByteDistributor propagate an INTERNAL_ERROR if an exception occurs during writing
- PriorityStreamByteDistributor to handle re-entry on the write method

Result:
PriorityStreamByteDistributor exception code state simplified, and re-entry is detected.
2015-11-03 13:22:11 -08:00
Norman Maurer
1b2e43e70c Correctly construct Executor in microbenchmarks.
Motivation:

We should allow our custom Executor to shutdown quickly.

Modifications:

Call super constructor which correct arguments.

Result:

Custom Executor can be shutdown quickly.
2015-11-03 09:46:05 +01:00
Trustin Lee
8f334885ef Reject the first SETTINGS ack on HTTP/2 Preface
Motivation:

Http2ConnectionHandler verifies if the first frame after the preface is
a SETTINGS frame.  However, it does not reject the SETTING ack frame
which is not expected actually.

Modifications:

Reject a SETTINGS-ack frame as well

Result:

When the first frame is a SETTINGS-ack frame, connection does not
proceed to further frame handling. (simplicity)
2015-11-03 11:23:54 +09:00
Scott Mitchell
c7cb104dc4 EPOLL Shutdown and Half Closed
Motivation:
The EPOLL module was not completly respecting the half closed state. It may have missed events, or procssed events when it should not have due to checking isOpen instead of the appropriate shutdown state.

Modifications:
- use FileDescriptor's isShutdown* methods instead of isOpen to check for processing events.

Result:
Half closed code in EPOLL module is more correct.
2015-11-02 13:01:24 -08:00
Scott Mitchell
dbbdbe11a6 Decouple Unix from Linux in Native Transport
Motivation:
transport-native-epoll is designed to be specific to Linux. However there is native code that can be extracted out and made to work on more Unix like distributions. There are a few steps to be completely decoupled but the first step is to extract out code that can run in a more general Unix environment from the Linux specific code base.

Modifications:
- Move all non-Linux specific stuff from Native.java into the io.netty.channel.unix package.
- io.netty.channel.unix.FileDescriptor will inherit all the native methods that are specific to file descriptors.
- io_netty_channel_epoll_Native.[c|h] will only have code that is specific to Linux.

Result:
Code is decoupled and design is streamlined in FileDescriptor.
2015-11-02 12:29:44 -08:00
Scott Mitchell
19658e9cd8 HTTP/2 Headers Type Updates
Motivation:
The HTTP/2 RFC (https://tools.ietf.org/html/rfc7540#section-8.1.2) indicates that header names consist of ASCII characters. We currently use ByteString to represent HTTP/2 header names. The HTTP/2 RFC (https://tools.ietf.org/html/rfc7540#section-10.3) also eludes to header values inheriting the same validity characteristics as HTTP/1.x. Using AsciiString for the value type of HTTP/2 headers would allow for re-use of predefined HTTP/1.x values, and make comparisons more intuitive. The Headers<T> interface could also be expanded to allow for easier use of header types which do not have the same Key and Value type.

Motivation:
- Change Headers<T> to Headers<K, V>
- Change Http2Headers<ByteString> to Http2Headers<CharSequence, CharSequence>
- Remove ByteString. Having AsciiString extend ByteString complicates equality comparisons when the hash code algorithm is no longer shared.

Result:
Http2Header types are more representative of the HTTP/2 RFC, and relationship between HTTP/2 header name/values more directly relates to HTTP/1.x header names/values.
2015-10-30 15:29:44 -07:00
Trustin Lee
07d861c5ff Fix a bug where DnsNameResolver attempts to send extra queries
Motivation:

When DnsNameResolverContext succeeds to get the address(es), it cancels
the promise of other queries in progress.

Unlike expectation, DnsNameResolverContext.query() attempts to retry
even when the query has failed due to cancellation.

As a result, the resolver sends unnecessary extra queries to a DNS
server and then tries to mark the promised that's been fulfilled
already, leading to unnecessarily verbose 'failed to notify success to a
promise' messages.

Modifications:

Do not perform an extra query when the previous query has failed due to
cancellation

Result:

DnsNameResolver does not send unnecessary extra queries and thus does
not log the 'failed to notify success to a promise' message.
2015-10-30 08:16:24 +01:00
Trustin Lee
d0f3cd383d Fix a bug where DefaultPromise.toString() says 'incomplete' when it's done
Motivation:

DefaultPromise.toString() returns 'DefaultPromise(incomplete)' when it's
actually complete with non-null result.

Modifications:

Handle the case where the promise is done and its result is non-null in
toString()

Result:

The String returned by DefaultPromise.toString() is not confusing
anymore.
2015-10-30 08:12:18 +01:00
buchgr
c9364616c8 Fix performance regression in FastThreadLocal microbenchmark. Fixes #4402
Motivation:

As reported in #4402, the FastThreadLocalBenchmark shows that the JDK ThreadLocal
is actually faster than Netty's custom thread local implementation.

I was looking forward to doing some deep digging, but got disappointed :(.

Modifications:

The microbenchmark was not using FastThreadLocalThreads and would thus always hit the slow path.
I updated the JMH command line flags, so that FastThreadLocalThreads would be used.

Result:

FastThreadLocalBenchmark shows FastThreadLocal to be faster than JDK's ThreadLocal implementation,
by about 56% in this particular benchmark. Run on OSX El Capitan with OpenJDK 1.8u60.

Benchmark                                    Mode  Cnt      Score      Error  Units
FastThreadLocalBenchmark.fastThreadLocal    thrpt   20  55452.027 ±  725.713  ops/s
FastThreadLocalBenchmark.jdkThreadLocalGet  thrpt   20  35481.888 ± 1471.647  ops/s
2015-10-29 21:40:13 +01:00
Norman Maurer
2e36ac4594 Add benchmark for HeapByteBuf implementations.
Motivation:

To prove one implementation is faster as the other we should have a benchmark.

Modifications:

Add benchmark which benchmarks the unsafe and non-unsafe implementation of HeapByteBuf.

Result:

Able to compare speed of implementations easily.
2015-10-29 19:38:52 +01:00
Norman Maurer
94e907fa39 Only call ReferenceCountUtil.touch(...) if ResourceLeakDetection was enabled when Channel was created.
Motivation:

We should only call ReferenceCountUtil.touch(...) if needed as otherwise we pay the overhead of instanceof and cast
everytime.

Modifications:

Add boolean flag which indicates if touch(...) should be called.

Result:

Less overhead when leak detection is not enabled.
2015-10-29 19:23:27 +01:00
Norman Maurer
a47685b243 Use bitwise operation when sampling for resource leak detection.
Motivation:

Modulo operations are slow, we can use bitwise operation to detect if resource leak detection must be done while sampling.

Modifications:

- Ensure the interval is a power of two
- Use bitwise operation for sampling
- Add benchmark.

Result:

Faster sampling.
2015-10-29 19:18:44 +01:00
Scott Mitchell
ed98cd8200 DefaultPromise StackOverFlowException
Motivation:
When the ImmediateEventExecutor is in use it is possible to get a StackOverFlowException if when a promise completes a new listener is added to that promise.

Modifications:
- Protect against the case where LateListeners.run() smashes the stack.

Result:
Fixes https://github.com/netty/netty/issues/4395
2015-10-29 11:10:57 -07:00
Scott Mitchell
d66520db1b Http2ConnectionHandler.BaseBuilder exception cleanup
Motivation:
Http2ConnectionHandler.BaseBuilder is constructing objects which have 'close' methods, but is not calling these methods in the event of an exception.

Modifications:
- Objects which implement 'close' should have this method called if an exception is thrown and the build operation can not complete normally.

Result:
Objects are closed even if the build process encounters an error.
2015-10-29 11:09:51 -07:00
Stephane Landelle
c6474f9218 Exclude native transport related test and dependency when not running under Linux, close #4409
Motivation:

The build fails on OSX, due to it trying to pull in an epoll specific OSX dependency. See #4409.

Modifications:

* move netty-transport-native-epoll to linux profile
* exclude Http2FrameWriterBenchmark from compiler
* include Http2FrameWriterBenchmark back only in linux profile (please check)

Result:

Build succeeds on OSX.
2015-10-29 16:23:25 +01:00
Norman Maurer
2100a83e30 [#4363] Improve size calculation of messages when written from outside the EventLoop
Motiviation:

If a user writes from outside the EventLoop we increase the pending bytes of the outbound buffer before submitting the write request. This is done so the user can stop writing asap once the channel turns unwritable. Unfortunally this doesn't take the overhead of adding the task into the account and so it is very easy for an user to full up the task queue. Beside this we use a value of 0 for an unown message by default which is not ideal.

Modifications:

- port the message calculation we used in netty 3.x into AbstractChannelHandlerContext and so better calculate the overhead of a message that is submitted from outside the EventLoop
- change the default estimated size for an unknown message to 8.

Result:

Better behaviour when submiting writes from outside the EventLoop.
2015-10-28 21:54:31 +01:00
Sverker Abrahamsson
e121c68e0f Created RTSPEncoder and RTSPDecoder which are now common for both requests and responses to be able to handle both types of messages on the same channel.
Keep RTSPRequestEncoder, RTSPRequestDecoder, RTSPResponseEncoder and
RTSPResponseDecoder for backwards compatibility but they now just extends
the generic encoder/decoder and are markes as deprecated.

Renamed the decoder test, because the decoder is now generic. Added
testcase for when ANNOUNCE request is received from server.

Created testcases for encoder.

Mark abstract base classes RTSPObjectEncoder and RTSPObjectDecoder as
deprecated, that functionality is now in RTSPEncoder and RTSPDecoder.

Added annotation in RtspHeaders to suppress warnings about deprecation, no need when
whole class is deprecated.
2015-10-27 14:01:20 +01:00
Sylwester Lachiewicz
c068ae8d84 Fix the 'branches to look' section 2015-10-27 13:59:11 +01:00
Norman Maurer
d93f906255 [#4198] Fix race-condition when allocate from multiple-thread.
Motivation:

Fix a race condition that was introduced by f18990a8a5 that could lead to a NPE when allocate from the PooledByteBufAllocator concurrently by many threads.

Modifications:

Correctly synchronize on the PoolSubPage head.

Result:

No more race.
2015-10-27 07:39:42 +01:00