Commit Graph

181 Commits

Author SHA1 Message Date
Anuraag Agrawal
1f1a60ae7d Use Netty's DefaultPriorityQueue instead of JDK's PriorityQueue for scheduled tasks
Motivation:

`AbstractScheduledEventExecutor` uses a standard `java.util.PriorityQueue` to keep track of task deadlines. `ScheduledFuture.cancel` removes tasks from this `PriorityQueue`. Unfortunately, `PriorityQueue.remove` has `O(n)` performance since it must search for the item in the entire queue before removing it. This is fast when the future is at the front of the queue (e.g., already triggered) but not when it's randomly located in the queue.

Many servers will use `ScheduledFuture.cancel` on all requests, e.g., to manage a request timeout. As these cancellations will be happen in arbitrary order, when there are many scheduled futures, `PriorityQueue.remove` is a bottleneck and greatly hurts performance with many concurrent requests (>10K).

Modification:

Use netty's `DefaultPriorityQueue` for scheduling futures instead of the JDK. `DefaultPriorityQueue` is almost identical to the JDK version except it is able to remove futures without searching for them in the queue. This means `DefaultPriorityQueue.remove` has `O(log n)` performance.

Result:

Before - cancelling futures has varying performance, capped at `O(n)`
After - cancelling futures has stable performance, capped at `O(log n)`

Benchmark results

After - cancelling in order and in reverse order have similar performance within `O(log n)` bounds
```
Benchmark                                           (num)   Mode  Cnt       Score      Error  Units
ScheduledFutureTaskBenchmark.cancelInOrder            100  thrpt   20  137779.616 ± 7709.751  ops/s
ScheduledFutureTaskBenchmark.cancelInOrder           1000  thrpt   20   11049.448 ±  385.832  ops/s
ScheduledFutureTaskBenchmark.cancelInOrder          10000  thrpt   20     943.294 ±   12.391  ops/s
ScheduledFutureTaskBenchmark.cancelInOrder         100000  thrpt   20      64.210 ±    1.824  ops/s
ScheduledFutureTaskBenchmark.cancelInReverseOrder     100  thrpt   20  167531.096 ± 9187.865  ops/s
ScheduledFutureTaskBenchmark.cancelInReverseOrder    1000  thrpt   20   33019.786 ± 4737.770  ops/s
ScheduledFutureTaskBenchmark.cancelInReverseOrder   10000  thrpt   20    2976.955 ±  248.555  ops/s
ScheduledFutureTaskBenchmark.cancelInReverseOrder  100000  thrpt   20     362.654 ±   45.716  ops/s
```

Before - cancelling in order and in reverse order have significantly different performance at higher queue size, orders of magnitude worse than the new implementation.
```
Benchmark                                           (num)   Mode  Cnt       Score       Error  Units
ScheduledFutureTaskBenchmark.cancelInOrder            100  thrpt   20  139968.586 ± 12951.333  ops/s
ScheduledFutureTaskBenchmark.cancelInOrder           1000  thrpt   20   12274.420 ±   337.800  ops/s
ScheduledFutureTaskBenchmark.cancelInOrder          10000  thrpt   20     958.168 ±    15.350  ops/s
ScheduledFutureTaskBenchmark.cancelInOrder         100000  thrpt   20      53.381 ±    13.981  ops/s
ScheduledFutureTaskBenchmark.cancelInReverseOrder     100  thrpt   20  123918.829 ±  3642.517  ops/s
ScheduledFutureTaskBenchmark.cancelInReverseOrder    1000  thrpt   20    5099.810 ±   206.992  ops/s
ScheduledFutureTaskBenchmark.cancelInReverseOrder   10000  thrpt   20      72.335 ±     0.443  ops/s
ScheduledFutureTaskBenchmark.cancelInReverseOrder  100000  thrpt   20       0.743 ±     0.003  ops/s
```
2017-11-10 23:09:32 -08:00
Trask Stalnaker
58e74e9fee Support running Netty in bootstrap class loader
Motivation:

Fix NullPointerExceptions that occur when running netty-tcnative inside the bootstrap class loader.

Modifications:

- Replace loader.getResource(...) with ClassLoader.getSystemResource(...) when loader is null.
- Replace loader.loadClass(...) with Class.forName(..., false, loader) which works when loader is both null and non-null.

Result:

Support running native libs in bootstrap class loader
2017-10-29 13:13:19 +01:00
Idel Pivnitskiy
4793daa589 Make Comparators Serializable
Motivation:

Objects of java.util.TreeMap or java.util.TreeSet will become
non-Serializable if instantiated with Comparators, which are not also
 Serializable. This can result in unexpected and difficult-to-diagnose
 bugs.

Modifications:

Implements Serializable for all classes, which implements Comparator.

Result:

Proper Comparators which will not force collections to
non-Serializable mode.
2017-10-22 03:40:28 +02:00
Idel Pivnitskiy
50a067a8f7 Make methods 'static' where it possible
Motivation:

Even if it's a super micro-optimization (most JVM could optimize such
 cases in runtime), in theory (and according to some perf tests) it
 may help a bit. It also makes a code more clear and allows you to
 access such methods in the test scope directly, without instance of
 the class.

Modifications:

Add 'static' modifier for all methods, where it possible. Mostly in
test scope.

Result:

Cleaner code with proper 'static' modifiers.
2017-10-21 14:59:26 +02:00
Norman Maurer
4d5f0e7ad5 NativeLibraryLoader should check the result of ClassLoader#getResource method
Motivation:

NativeLibraryLoader uses ClassLoader#getResource method that can return nulls when the resource cannot be found. The returned url variable should be checked for nullity and fail in a more usable manner than a NullPointerException

Modifications:

Fail with a FileNotFoundException

Result:

Fixes [#7222].
2017-09-19 17:45:06 -07:00
杨浩
14189140a0 log in PatternLayout (%F:%L)%c.%M
Motivation:

When Log4j2Logger is used with PatternLayout (%F:%L)%c.%M, the log message incorrect shows:

(Log4J2Logger.java:73)io.netty.util.internal.PlatformDependent0.debug ....

Modification:

Extend AbstractLogger

Result:

Fixes [#7186].
2017-09-14 14:51:20 -07:00
Nikolay Fedorovskikh
01eb428b39 Move methods for decode hex dump into StringUtil
Motivation:

PR #6811 introduced a public utility methods to decode hex dump and its parts, but they are not visible from netty-common.

Modifications:

1. Move the `decodeHexByte`, `decodeHexDump` and `decodeHexNibble` methods into `StringUtils`.
2. Apply these methods where applicable.
3. Remove similar methods from other locations (e.g. `HpackHex` test class).

Result:

Less code duplication.
2017-06-23 18:52:42 +02:00
Nikolay Fedorovskikh
aa38b6a769 Prevent unnecessary allocations in the StringUtil#escapeCsv
Motivation:

A `StringUtil#escapeCsv` creates new `StringBuilder` on each value even if the same string is returned in the end.

Modifications:

Create new `StringBuilder` only if it really needed. Otherwise, return the original string (or just trimmed substring).

Result:

Less GC load. Up to 4x faster work for not changed strings.
2017-06-13 14:57:38 -07:00
Nikolay Fedorovskikh
d768c5e628 MessageFormatter improvements
Motivation:

`FormattingTuple.getArgArray()` is never used.
In the `MessageFormatter` it is possible to make
some improvements, e.g. replace `StringBuffer`
with `StringBuilder`, avoid redundant allocations, etc.

Modifications:

- Remove `argArray` field from the `FormattingTuple`.
- In `MessageFormatter`:
  - replace `StringBuffer` with `StringBuilder`,
  - replace `HashMap` with `HashSet` and make it lazy initialized.
  - avoid redundant allocations (`substring()`, etc.)
  - use appropriate StringBuilder's methods for the some `Number` values.
- Porting unit tests from `slf4j`.

Result:

Less GC load on logging with internal `MessageFormatter`.
2017-05-19 09:47:11 -07:00
Nikolay Fedorovskikh
e4531918a3 Optimizations in NetUtil
Motivation:

IPv4/6 validation methods use allocations, which can be avoided.
IPv4 parse method use StringTokenizer.

Modifications:

Rewriting IPv4/6 validation methods to avoid allocations.
Rewriting IPv4 parse method without use StringTokenizer.

Result:

IPv4/6 validation and IPv4 parsing faster up to 2-10x.
2017-05-18 16:42:22 -07:00
Norman Maurer
0ee49e6d66 Eliminate noisy logging when using sun.misc.Unsafe and running on pre Java9
Motivation:

We should only try to load jdk.internal.misc.Unsafe if we run on Java9+ to eliminate noise in the log.

Modifications:

- Move javaVersion() and related methods to PlatformDependent0 to be able to use these in the static initializer without creating a cycle.
- Only try to load jdk.internal.misc.Unsafe when running in Java9+

Result:

Less noise in the log when running pre java9.
2017-05-16 08:29:06 +02:00
Nikolay Fedorovskikh
5643cc6a10 IPv6 validation fixes
Motivation:

`NetUtil`'s methods `isValidIpV6Address` and `getIPv6ByName` incorrectly validate some IPv6 addresses.

Modifications:

- `getIPv6ByName`: add checks for single colon at the start or end.
- `isValidIpV6Address`: fix checks for the count of colons and use `endOffset` instead of `ipAddress.length()` for the cases with the brackets or '%'.

Result:

More correct implementation of `NetUtil#isValidIpV6Address` and `NetUtil#getIPv6ByName`.
2017-05-11 08:10:25 -07:00
Vladimir Kostyukov
ed37cf20ef Introduce HashedWheelTimer.pendingTimeouts()
Motivation:

Fixes #6681.

Modification:

For the sake of better timer observability, expose the number of pending timeouts through the new HashedWheelTimer.pendingTimeouts method .

Result:

It's now ridiculously easy to observe Netty timer's very basic and yet important metric, the number of pending tasks/timeouts.
2017-05-01 20:10:22 -07:00
Scott Mitchell
ea1cb20c90 Netutil IPv6 bugs
Motivation:
NetUtil#isValidIpV6Address and NetUtil#getIPv6ByName allowed an invalid form of mapped IPv4 addresses which lead to accepting invalid IPv6 addresses as valid.

Modifications:
- NetUtil#isValidIpV6Address and NetUtil#getIPv6ByName should only allow 7 colons for an IPv4 address if they are the first 2 characters.

Result:
More correct implementation of NetUtil#isValidIpV6Address and NetUtil#getIPv6ByName
2017-04-28 07:29:36 +02:00
Scott Mitchell
9cb858fcf6 NetUtil IPv6 bugs related to IPv4 and compression
Motivation:
NetUtil#getByName and NetUtil#isValidIpV6Address do not strictly enforce the format of IPv4 addresses that are allowed to be embedded in IPv6 addresses as specified in https://tools.ietf.org/html/rfc4291#section-2.5.5. This may lead to invalid addresses being parsed, or invalid addresses being considered valid. Compression of a single IPv6 word was also not handled correctly if there are 7 : characters.

Modifications:
- NetUtil#isValidIpV6Address should enforce the IPv4-Compatible and IPv4-Mapped are the only valid formats for including IPv4 addresses as specified in https://tools.ietf.org/html/rfc4291#section-2.5.5
- NetUtil#getByName should more stritcly parse IPv6 addresses which contain IPv4 addresses as specified in https://tools.ietf.org/html/rfc4291#section-2.5.5
- NetUtil should allow compression even if the number of : characters is 7.
- NetUtil#createByteArrayFromIpAddressString should use the same IP string to byte[] translation which is used in NetUtil#getByName

Result:
NetUtil#getByName and NetUtil#isValidIpV6Address respect the IPv6 RFC which defines the valid formats for embedding IPv4 addresses.
2017-04-25 15:10:38 -07:00
Jason Tedor
98beb777f8 Enable configuring available processors
Motivation:

In cases when an application is running in a container or is otherwise
constrained to the number of processors that it is using, the JVM
invocation Runtime#availableProcessors will not return the constrained
value but rather the number of processors available to the virtual
machine. Netty uses this number in sizing various resources.
Additionally, some applications will constrain the number of threads
that they are using independenly of the number of processors available
on the system. Thus, applications should have a way to globally
configure the number of processors.

Modifications:

Rather than invoking Runtime#availableProcessors, Netty should rely on a
method that enables configuration when the JVM is started or by the
application. This commit exposes a new class NettyRuntime for enabling
such configuraiton. This value can only be set once. Its default value
is Runtime#availableProcessors so that there is no visible change to
existing applications, but enables configuring either a system property
or configuring during application startup (e.g., based on settings used
to configure the application).

Additionally, we introduce the usage of forbidden-apis to prevent future
uses of Runtime#availableProcessors from creeping. Future work should
enable the bundled signatures and clean up uses of deprecated and
other forbidden methods.

Result:

Netty can be configured to not use the underlying number of processors,
but rather the constrained number of processors.
2017-04-23 10:31:17 +02:00
Nikolay Fedorovskikh
0692bf1b6a fix the typos 2017-04-20 04:56:09 +02:00
Norman Maurer
1b0b8f80cd AbstractScheduledEventExecutor.schedule(...) must accept delay <= 0.
Motivation:

As the javadoc of ScheduledExecutorService state:

Zero and negative delays (but not periods) are also allowed in schedule methods,and are treated as requests for immediate execution.

Modifications:

- Correctly handle delay <= 0.
- Add unit tests.

Result:

Fixes [#6627].
2017-04-19 11:35:50 +02:00
Norman Maurer
493a8135f8 Ensure test introduced in 5c1c14286d also works on Java9 2017-03-29 22:43:00 +02:00
Norman Maurer
5c1c14286d Allow negative memoryAddress when calling PlatformDependent0.newDirectBuffer(...)
Motivation:

When UNSAFE.allocateMemory is returning an address whose high bit is set we currently throw an IllegalArgumentException. This is not correct as it may return a negative number on at least sparc.

Modifications:

- Allow to pass in negative memoryAddress
- Add unit tests

Result:

Correctly validate the memoryAddress and so also work on sparc as expected. Fixes [#6574].
2017-03-29 22:33:34 +02:00
David Dossot
9c1a191696 Trim optional white space in CombinedHttpHeaders values
Motivation:

The updated HTTP/1.x RFC allows for header values to be CSV and separated by OWS [1]. CombinedHttpHeaders should remove this OWS on insertion.

[1] https://tools.ietf.org/html/rfc7230#section-7

Modification:

CombinedHttpHeaders doesn't account for the OWS and returns it back to the user as part of the value.

Result:

Fixes #6452
2017-03-19 08:17:29 -07:00
Norman Maurer
e12f504ac1 Remove deprecated usage of Mockito methods
Motivation:

We used some deprecated Mockito methods.

Modifications:

- Replace deprecated method usage
- Some cleanup

Result:

No more usage of deprecated Mockito methods. Fixes [#6482].
2017-03-09 20:59:54 +01:00
Norman Maurer
c6a3cae269 UnorderedThreadPoolEventExecutor consumes 100% CPU when idle
Motivation:

When UnorderedThreadPoolEventExecutor.execute / submit etc is called it will consume up to 100 % CPU even after the task was executed.

Modifications:

Add a special wrapper which we will be used in execute(...) to wrap the submitted Runnable. This is needed as  ScheduledThreadPoolExecutor.execute(...) will delegate to submit(...) which will then use decorateTask(...). The problem with this is that decorateTask(...) needs to ensure we only do our own decoration if we not call from execute(...) as otherwise we may end up creating an endless loop because DefaultPromise will call  EventExecutor.execute(...) when notify the listeners of the promise.

Result:

Fixes [#6507].
2017-03-09 11:12:42 +01:00
Nikolay Fedorovskikh
2993760e92 Fix misordered 'assertEquals' arguments in tests
Motivation:

Wrong argument order in some 'assertEquals' applying.

Modifications:

Flip compared arguments.

Result:

Correct `assertEquals` usage.
2017-03-08 22:48:37 -08:00
Norman Maurer
a7c0ff665c Only use Mockito for mocking.
Motivation:

We used various mocking frameworks. We should only use one...

Modifications:

Make usage of mocking framework consistent by only using Mockito.

Result:

Less dependencies and more consistent mocking usage.
2017-02-07 08:47:22 +01:00
Jason Tedor
42c0359820 Do not prefer empty MAC address
Motivation:

When comparing MAC addresses searching for the best MAC address, if
locally-administered address (e.g., from a Docker container) is compared
against an empty MAC address, the empty MAC address will be marked as
preferred. In cases this is the only available MAC address, this leaves
Netty using a random machine ID instead of using a perfectly valid
machine ID from the locally-adminstered address.

Modifications:

This commit modifies the MAC address logic so that the empty MAC address
is not preferred over a locally-administered address. This commit also
simplifies the comparison logic here.

Result:

Empty MAC addresses will not be preferred over locally-administered
addresses thus permitting the default machine ID to be the
locally-adminstered MAC address if it is the only available MAC address.
2017-02-06 12:14:27 -08:00
Scott Mitchell
3482651e0c HTTP/2 Non Active Stream RFC Corrections
Motivation:
codec-http2 couples the dependency tree state with the remainder of the stream state (Http2Stream). This makes implementing constraints where stream state and dependency tree state diverge in the RFC challenging. For example the RFC recommends retaining dependency tree state after a stream transitions to closed [1]. Dependency tree state can be exchanged on streams in IDLE. In practice clients may use stream IDs for the purpose of establishing QoS classes and therefore retaining this dependency tree state can be important to client perceived performance. It is difficult to limit the total amount of state we retain when stream state and dependency tree state is combined.

Modifications:
- Remove dependency tree, priority, and weight related items from public facing Http2Connection and Http2Stream APIs. This information is optional to track and depends on the flow controller implementation.
- Move all dependency tree, priority, and weight related code from DefaultHttp2Connection to WeightedFairQueueByteDistributor. This is currently the only place which cares about priority. We can pull out the dependency tree related code in the future if it is generally useful to expose for other implementations.
- DefaultHttp2Connection should explicitly limit the number of reserved streams now that IDLE streams are no longer created.

Result:
More compliant with the HTTP/2 RFC.
Fixes https://github.com/netty/netty/issues/6206.

[1] https://tools.ietf.org/html/rfc7540#section-5.3.4
2017-02-01 10:34:27 -08:00
Norman Maurer
eb5dc4bced Correctly handle IPV6 in HttpProxyHandler
Motivation:

The HttpProxyHandler is expected to be capable of issuing a valid CONNECT request for a tunneled connection to an IPv6 host.

Modifications:

- Correctly format the IPV6 address.
- Add unit tests

Result:

HttpProxyHandler works with IPV6 as well. Fixes [#6152].
2017-01-12 07:51:37 +01:00
Norman Maurer
31da0ddbac Revert "Disallow setting logger factory twice"
This reverts commit 3c92f2b64a which needs more thoughts and so will go into the next release.
2016-12-21 15:14:53 +01:00
Scott Mitchell
3d11334151 Fix DefaultChannelId MAC address parsing bug
Motivation:
DefaultChannelId provides a regular expression which validates if a user provided MAC address is valid. This regular expression may allow invalid MAC addresses and also not allow valid MAC addresses.

Modifications:
- Introduce a MacAddressUtil#parseMac method which can parse and validate the MAC address at the same time. The regular expression check before hand is additional overhead if we have to parse the MAC address.

Result:
Fixes https://github.com/netty/netty/issues/6132.
2016-12-20 17:06:27 -08:00
Norman Maurer
f6ac8b5d32 [#6114] Increase test timeout for test introduced in c2f4daa739
Motivation:

c2f4daa739 added a unit test but used a too small test timeout.

Modifications:

Increase timeout.

Result:

Test should have enough time to complete on the CI.
2016-12-08 13:17:42 +01:00
Jason Tedor
3c92f2b64a Disallow setting logger factory twice
Motivation:

InternalLoggerFactory either sets a default logger factory
implementation based on the logging implementations on the classpath, or
applications can set a logger factory explicitly. If applications wait
too long to set the logger factory, Netty will have already set a logger
factory leading to some objects using one logging implementation and
other objets using another logging implementation. This can happen too
if the application tries to set the logger factory twice, which is
likely a bug in the application. Yet, the Javadocs for
InternalLoggerFactory warn against this saying that
InternalLoggerFactory#setLoggerFactory "should be called as early as
possible and shouldn't be called more than once". Instead, Netty should
guard against this.

Modications:

We replace the logger factory field with an atomic reference on which we
can do CAS operations to safely guard against it being set twice. We
also add an internal holder class that captures the static interface of
InternalLoggerFactory that can aid in testing.

Result:

The logging factory can not be set twice, and applications that want to
set the logging factory must do it before any Netty classes are
initialized (or the default logger factory will be set).
2016-12-08 10:38:01 +01:00
Norman Maurer
c2f4daa739 Fix false-positives when using ResourceLeakDetector.
Motivation:

We need to ensure the tracked object can not be GC'ed before ResourceLeak.close() is called as otherwise we may get false-positives reported by the ResourceLeakDetector. This can happen as the JIT / GC may be able to figure out that we do not need the tracked object anymore and so already enqueue it for collection before we actually get a chance to close the enclosing ResourceLeak.

Modifications:

- Add ResourceLeakTracker and deprecate the old ResourceLeak
- Fix some javadocs to correctly release buffers.
- Add a unit test for ResourceLeakDetector that shows that ResourceLeakTracker has not the problems.

Result:

No more false-positives reported by ResourceLeakDetector when ResourceLeakDetector.track(...) is used.
2016-12-04 09:01:39 +01:00
Scott Mitchell
00fc239995 HashWheelTimerTest cleanup
Motivation:
HashWheelTimerTest has busy/wait and sleep statements which are not necessary. We also depend upon a com.google.common.base.Supplier which isn't necessary.

Modifications:
- Remove buys wait loops and timeouts where possible

Result:
HashWheelTimerTest more explicit in verifying conditions and less reliant on wait times.
2016-11-14 16:17:18 -08:00
Aniket Bhatnagar
3f20b8adee Added optional pending timeouts counter parameter to HashedWheelTimer constructor and ensured that pending timeouts don't exceed provided max pending timeouts.
Motivation:
If the rate at which new timeouts are created is very high and the created timeouts are not cancelled, then the JVM can crash because of out of heap space. There should be a guard in the implementation to prevent this.

Modifications:
The constructor of HashedWheelTimer now takes an optional max pending timeouts parameter beyond which it will reject new timeouts by throwing RejectedExecutionException.

Result:
After this change, if the max pending timeouts parameter is passed as constructor argument to HashedWheelTimer, then it keeps a track of pending timeouts that aren't yet expired or cancelled. When a new timeout is being created, it checks for current pending timeouts and if it's equal to or greater than provided max pending timeouts, then it throws RejectedExecutionException.
2016-11-09 10:58:35 +01:00
Norman Maurer
273778c96c Correctly add futures to list in test
Motivation:

We missed to add the futures to the list in the test.

Modifications:

Add futures to list.

Result:

More correct test.
2016-11-01 10:23:03 +01:00
radai-rosenblatt
15ac6c4a1f Clean-up unused imports
Motivation:

the build doesnt seem to enforce this, so they piled up

Modifications:

removed unused import lines

Result:

less unused imports

Signed-off-by: radai-rosenblatt <radai.rosenblatt@gmail.com>
2016-09-30 09:08:50 +02:00
Scott Mitchell
506ac2ca71 NetUtil.bytesToIpAddress bug
Motivation:
NetUtil.bytesToIpAddress does not correctly translate IPv4 address to String. Also IPv6 addresses may not follow minimization conventions when converting to a String (see rfc 5952).

Modifications:
- NetUtil.bytesToIpAddress should correctly handle negative byte values for IPv4
- NetUtil.bytesToIpAddress should leverage existing to string conversion code in NetUtil

Result:
Fixes https://github.com/netty/netty/issues/5821
2016-09-22 17:06:21 -07:00
Norman Maurer
d44017189e Remove extra conditional check in retain
Motivation:

We not need to do an extra conditional check in retain(...) as we can just check for overflow after we did the increment.

Modifications:

- Remove extra conditional check
- Add test code.

Result:

One conditional check less.
2016-08-05 13:09:26 +02:00
Norman Maurer
fc14ca31cb Add NonStickyEventExecutorGroup
Motivation:

We offer DefaultEventExecutorGroup as an EventExecutorGroup which return OrderedEventExecutor and so provide strict ordering of event execution. One limitations of this implementation is that each contained DefaultEventExecutor will always be tied to a single thread, which can lead to a very unbalanced execution as one thread may be super busy while others are idling.

Modifications:

- Add NonStickyEventExecutorGroup which can be used to wrap another EventExecutorGroup (like UnorderedThreadPoolEventExecutor) and expose ordering while not be sticky with the thread that is used for a given EventExecutor. This basically means that Threads may change between execution of tasks for an EventExecutor but ordering is still guaranteed.

Result:

Better utalization of threads in some use-cases.
2016-08-04 06:30:59 +02:00
Norman Maurer
d3dc9c9e74 Allow to limit the maximum number of WeakOrderQueue instances per Thread.
Motivation:

To better restrict resource usage we should limit the number of WeakOrderQueue instances per Thread. Once this limit is reached object that are recycled from a different Thread then the allocation Thread are dropped on the floor.

Modifications:

Add new system property io.netty.recycler.maxDelayedQueuesPerThread and constructor that allows to limit the max number of WeakOrderQueue instances per Thread for Recycler instance. The default is 2 * cores (the same as the default number of EventLoop instances per EventLoopGroup).

Result:

Better way to restrict resource / memory usage per Recycler instance.
2016-08-04 06:23:14 +02:00
Dmitriy Dumanskiy
a80ea46b8e Removed custom split method as it is not effective anymore. 2016-08-01 21:49:33 +02:00
Norman Maurer
d92c5f5f5b Introduce allocation / pooling ratio in Recycler
Motivation:

At the moment the Recyler is very sensitive to allocation bursts which means that if there is a need for X objects for only one time these will most likely end up in the Recycler and sit there forever as the normal workload only need a subset of this number.

Modifications:

Add a ratio which sets how many objects should be pooled for each new allocation. This allows to slowly increase the number of objects in the Recycler while not be to sensitive for bursts.

Result:

Less unused objects in the Recycler if allocation rate sometimes bursts.
2016-07-29 15:20:39 +02:00
Norman Maurer
d315f1b3ba [#5551] Fix sporadic GlobalEventExecutorTest.testAutomaticStartStop test failure
Motivation:

We saw some sporadic test failures for GlobalEventExecutorTest.testAutomaticStartStop test. This is caused parallel execution of tests in combination with assert checks that will be affected.

Modifications:

Remove fragile assert checks.

Result:

No more sporadic test failures
2016-07-25 08:26:30 +02:00
Jason Tedor
e00b797936 Acquire Java version simply
Motivation:

The Java version is used for platform dependent logic. Yet, the logic
for acquiring the Java version requires special permissions (the runtime
permission "getClassLoader") that some downstream projects will never
grant. As such, these projects are doomed to have Netty act is their
Java major version is six.  While there are ways to maintain the same
logic without requiring these special permissions, the logic is
needlessly complicated because it relies on loading classes that exist
in version n but not version n - 1. This complexity can be removed. As a
bonanza, the dangerous permission is no longer required.

Modifications:

Rather than attempting to load classes that exist in version n but not
in version n - 1, we can just parse the Java specification version. This
only requires a begign property (property permission
"java.specification.version") and is simple.

Result:

Acquisition of the Java version is safe and simple.
2016-07-19 18:51:25 +02:00
Jason Tedor
27520f5208 Non-sticky thread groups in DefaultThreadFactory
Motivation:

A recent change to DefaultThreadFactory modified it so that it is sticky
with respect to thread groups. In particular, this change made it so
that DefaultThreadFactory would hold on to the thread group that created
it, and then use that thread group to create threads.

This can have problematic semantics since it can lead to violations of a
tenet of thread groups that a thread can only modify threads in its own
thread group and descendant thread groups. With a sticky thread group, a
thread triggering the creation of a new thread via
DefaultThreadFactory#newThread will be modifying a thread from the
sticky thread group which will not necessarily be its own nor a
descendant thread group. When a security manager is in place that
enforces this requirement, these modifications are now impossible. This
is especially problematic in the context of Netty because certain global
singletons like GlobalEventExecutor will create a
DefaultThreadFactory. If all DefaultThreadFactory instances are sticky
about their thread groups, it means that submitting tasks to the
GlobalEventExecutor singleton can cause a thread to be created from the
DefaultThreadFactory sticky thread group, exactly the problem with
DefaultThreadFactory being sticky about thread groups. A similar problem
arises from the ThreadDeathWatcher.

Modifications:

This commit modifies DefaultThreadFactory so that a null thread group
can be set with the behavior that all threads created by such an
instance will inherit the default thread group (the thread group
provided by the security manager if there is one, otherwise the thread
group of the creating thread). The construction of the instances of
DefaultThreadFactory used by the GlobalEventExecutor singleton and
ThreadDeathWatcher are modified to use this behavior. Additionally, we
also modify the chained constructor invocations of the
DefaultThreadFactory that do not have a parameter to specify a thread
group to use the thread group from the security manager is available,
otherwise the creating thread's thread group. We also add unit tests
ensuring that all of this behavior is maintained.

Result:

It will be possible to have DefaultThreadFactory instances that are not
sticky about the thread group that led to their creation. Instead,
threads created by such a DefaultThreadFactory will inherit the default
thread group which will either be the thread group from the security
manager or the current thread's thread group.
2016-07-14 22:06:40 +02:00
Norman Maurer
afafadd3d7 [#5505] Enforce Recycler limit when recycling from different threads
Motivation:

Currently, the recycler max capacity it's only enforced on the
thread-local stack which is used when the recycling happens on the
same thread that requested the object.

When the recycling happens in a different thread, then the objects
will be queued into a linked list (where each node holds N objects,
default=16). These objects are then transfered into the stack when
new objects are requested and the stack is empty.

The problem is that the queue doesn't have a max capacity and that
can lead to bad scenarios. Eg:

- Allocate 1M object from recycler
- Recycle all of them from different thread
- Recycler WeakOrderQueue will contain 1M objects
- Reference graph will be very long to traverse and GC timeseems to be negatively impacted
- Size of the queue will never shrink after this

Modifications:

Add some shared counter which is used to manage capacity limits when recycle from different thread then the allocation thread. We modify the counter whenever we allocate a new Link to reduce the overhead of increment / decrement it.

Result:

More predictable number of objects mantained in the recycler pool.
2016-07-14 07:56:03 +02:00
Jason Tedor
cfd6db7915 Avoid missed signals on a default promise
Motivation:

Today when awaiting uninterruptibly on a default promise, a race
condition can lead to a missed signal. Quite simply, the check for
whether the condition holds is not made inside a lock before
waiting. This means that the waiting thread can enter the wait after the
promise has completed and will thus not be notified, thus missing the
signal. This leads to the waiting thread to enter a timed wait that will
only trip with the timeout elapses leading to unnecessarily long waits
(imagine a connection timeout, and the waiting thread missed the signal
that the connection is ready).

Modification:

This commit fixes this missed signal by checking the condition inside a
lock. We also add a test that reliably fails without the non-racy
condition check.

Result:

Timed uninterruptible waits on default promise will not race against the
condition and possibly wait longer than necessary.
2016-07-12 09:57:14 +02:00
Norman Maurer
ce95c50444 [#5507] SingleThreadEventExecutor should reject call invoke*() from within the EventLoop.
Motivation:

ExecutorService.invoke*(...) methods may block by API definition. This can lead to deadlocks if called from inside the EventLoop in SingleThreadEventExecutor as it only has one Thread that does all the work.

Modifications:

Throw a RejectedExectionException if someone tries to call SingleThreadEventExecutor.invoke*(...) while in the EventLoop.

Result:

No more deadlock possible.
2016-07-09 08:08:50 +02:00
Julien Viet
79c8ec4d33 DnsNameResolver search domains support
Motivation:

The current DnsNameResolver does not support search domains resolution. Search domains resolution is supported out of the box by the java.net resolver, making the DnsNameResolver not able to be a drop in replacement for io.netty.resolver.DefaultNameResolver.

Modifications:

The DnsNameResolverContext resolution has been modified to resolve a list of search path first when it is configured so. The resolve method now uses the following algorithm:

if (hostname is absolute (start with dot) || no search domains) {
 searchAsIs
} else {
  if (numDots(name) >= ndots) {
    searchAsIs
  }
  if (searchAsIs wasn't performed or failed) {
    searchWithSearchDomainsSequenciallyUntilOneSucceeds
  }
}

The DnsNameResolverBuilder provides configuration for the search domains and the ndots value. The default search domains value is configured with the OS search domains using the same native configuration the java.net resolver uses.

Result:

The DnsNameResolver performs search domains resolution when they are present.
2016-07-08 22:04:01 +02:00