Commit Graph

9084 Commits

Author SHA1 Message Date
Norman Maurer
0ddc62cec0
Add support for TLSv1.3 (#8293)
Motivation:

TLSv1.3 support is included in java11 and is also supported by OpenSSL 1.1.1, so we should support when possible.

Modifications:
- Add support for TLSv1.3 using either the JDK implementation or the native implementation provided by netty-tcnative when compiled against openssl 1.1.1
- Adjust unit tests for semantics provided by TLSv1.3
- Correctly handle custom Provider implementations that not support TLSv1.3

Result:

Be able to use TLSv1.3 with netty.
2018-10-17 08:35:35 +02:00
Stephane Landelle
9eebe7ed74 Add full JdkSslContext public constructor, close #8384 (#8389)
Motivation:

JdkSslContext provides public constructors to wrap an existing `javax.net.ssl.SSLContext`.

Sadly, some options combinations are not possible with the existing constructors, eg:
*  protocols is not exposed and always forced to null, so default protocols are always enforced
* startTls is not exposed and always forced to false

Modification:

Add full constructor that take protocols and startTls parameters.

Result:

It's possible to create a JdkSslContext from an existing SSLContext and still have control over protocols and startTls
2018-10-17 08:28:39 +02:00
Julien Hoarau
04001fdad1 Upgrade h2spec-maven-plugin (#8386)
Motivation:
- Version 0.3 would sometimes fail to report failing tests
- New version contains upgraded version of h2spec

Modifications:
- Bump h2spec-maven-plugin to 0.6
- Remove excluded specs that are no passing
- Add failing spec "half closed (remote): Sends a HEADERS frame" to
exclude list

Result:
Build will fail when non excluded specs fails.
2018-10-16 07:05:45 +02:00
Dmitriy Dumanskiy
b59336142f deprecate own ConcurrentSet for removal (#8340)
Motivation:

Java since version 6 has the wrapper for the ConcurrentHashMap that could be created via Collections.newSetFromMap(map). So no need to create own ConcurrentSet class. Also, since netty plans to switch to Java 8 soon there is another method for that - ConcurrentHashMap.newKeySet().
For now, marking this class @deprecated would be enough, just to warn users who use netty's ConcurrentSet. After switching to Java 8 ConcurrentSet should be removed and replaced with ConcurrentHashMap.newKeySet().

Modification:

ConcurrentSet deprecated.
2018-10-15 19:36:05 +02:00
Norman Maurer
a542d4d78b
Increase test timeout (#8385)
Motivation:

It has shown that the used test timeout may be too low when the CI is busy.

Modifications:

Increase timeout to 3 seconds.

Result:

Less false-positives.
2018-10-15 15:10:22 +02:00
Norman Maurer
4a2d03542b Exclude mainframer related files from git 2018-10-14 13:20:18 +02:00
Nick Hill
7062ceedb0 Simplify ByteBufInputStream.readLine() logic (#8380)
Motivation:

While looking at the nice optimization done in
https://github.com/netty/netty/pull/8347 I couldn't help noticing the
logic could be simplified further. Apologies if this is just my OCD and
inappropriate!

Modifications:

Reduce amount of code used for ByteBufInputStream.readLine()

Result:

Slightly smaller and simpler code
2018-10-13 06:24:40 +02:00
Norman Maurer
adb4ce1f31
Use MACOSX_DEPLOYMENT_TARGET=10.6 when compile native transport on MacOS (#8379)
Motivation:

MACOSX_DEPLOYMENT_TARGET=10.6 needs to be used as everything before is not supported in 10.14 anymore. 10.6 was released 2009 so this should be a safe thing to do.

Modifications:

Use MACOSX_DEPLOYMENT_TARGET=10.6

Result:

Be able to compile on MacOS 10.14
2018-10-13 04:25:56 +02:00
Norman Maurer
aae7cdca96
Prevent NPE when attempting to set client key material with no alias (#8378)
Motivation:

It is possible that a client is unable to locate a certificate alias given the list of issuers and key types. In this case the X509KeyManager
will return a null which when past to the OpenSslKeyMaterialProvider implementation may produce a NPE. If no matching alias could be found we should not
call OpenSslKeyMaterialProvider at all which is also consistent what OpenJDK does.

Modifications:

- Add null check before calling OpenSslKeyMaterialProvider
- Add unit test.

Result:

No more NPE caused by passing null as client alias.
2018-10-12 09:27:46 +02:00
Johno Crawford
5b3b8db07f epoll_wait produces an EINVAL error since 4.1.30 (#8350)
Motivation:

epoll_wait should work in 4.1.30 like it did in 4.1.29.

Modifications:

Revert Integer.MAX_VALUE back to MAX_SCHEDULED_TIMERFD_NS (999,999,999).
Add unit test.

Result:

epoll_wait will no longer throw EINVAL.
2018-10-12 05:02:41 +02:00
Norman Maurer
2a4bb346cf
Correctly parse /etc/resolv.conf when contain multiple entries for searchdomain. (#8351)
Motivation:

ba594bcf4a added a utility to parse searchdomains defined in /etc/resolv.conf but did not correctly handle the case when multiple are defined that are seperated by either whitespace or tab.

Modifications:

- Correctly parse multiple entries
- Add unit test.

Result:

Correctly parse multiple searchdomain entries.
2018-10-12 05:00:32 +02:00
Norman Maurer
652650b0db
Correctly decrement pending bytes when submitting AbstractWriteTask fails. (#8349)
Motivation:

Currently we may end up in the situation that we incremented the pending bytes before submitting the AbstractWriteTask but never decrement these again if the submitting of the task fails. This may result in incorrect watermark handling.

Modifications:

- Correctly decrement pending bytes if subimitting of task fails and also ensure we recycle it correctly.
- Add unit test.

Result:

Fixes https://github.com/netty/netty/issues/8343.
2018-10-11 18:46:10 +02:00
Dmitriy Dumanskiy
0e4186c552 deprecate IntegerHolder for removal (#8339)
Motivation:

Seems like IntegerHolder counterHashCode field is the very old legacy field that is no longer used. Should be marked as deprecated and removed in the future versions.

Modification:

IntegerHolder class, InternalThreadLocalMap.counterHashCode() and InternalThreadLocalMap.setCounterHashCode(IntegerHolder counterHashCode) are now deprecated.
2018-10-11 14:59:47 +08:00
Francesco Nigro
83dc3b503e ByteBufInputStream is always allocating a StringBuilder instance (#8347)
Motivation:

Avoid creating any StringBuilder instance if
ByteBufInputStream::readLine isn't used

Modifications:

The StringBuilder instance is lazy allocated on demand and
are added new test case branches to address the increased
complexity of ByteBufInputStream::readLine

Result:

Reduced GC activity if ByteBufInputStream::readLine isn't used
2018-10-11 14:56:29 +08:00
Jussi Virtanen
fc28bccdf1 Fix SelectableChannel support in NioEventLoop (#8344)
Motivation:

Unless the 'io.netty.noKeySetOptimization' system property is set,
registering a SelectableChannel instance to a NioEventLoop results
in a ClassCastException:

    io.netty.channel.nio.SelectedSelectionKeySetSelector cannot be cast
        to java.nio.channels.spi.AbstractSelector

Modifications:

Instead of 'selector', pass 'unwrappedSelector' to SelectableChannel.

Result:

It is possible to register a SelectableChannel instance without
setting the 'io.netty.noKeySetOptimization' system property.
2018-10-09 12:11:07 +08:00
Dmitriy Dumanskiy
6cebb6069b remove unnecessary vararg argument in PooledByteBufAllocator (#8338)
Motivation:

No need in varargs, the method always accepts array.

Modification:

... replaced with []
2018-10-05 19:06:44 +08:00
Matteo Merli
3a96e7373b Added option to do busy-wait on epoll (#8267)
Motivation:

Add an option (through a SelectStrategy return code) to have the Netty event loop thread to do busy-wait on the epoll.

The reason for this change is to avoid the context switch cost that comes when the event loop thread is blocked on the epoll_wait() call.

On average, the context switch has a penalty of ~13usec.

This benefits both:

The latency when reading from a socket
Scheduling tasks to be executed on the event loop thread.
The tradeoff, when enabling this feature, is that the event loop thread will be using 100% cpu, even when inactive.

Modification:

Added SelectStrategy option to return BUSY_WAIT
Epoll loop will do a epoll_wait() with no timeout
Use pause instruction to hint to processor that we're in a busy loop
Result:

When enabled, minimizes impact of context switch in the critical path
2018-09-28 22:52:00 +02:00
Eric Anderson
a95b7a791e
Notify http2 error handler before closeStreamLocal on HEADERS write failure (#8332)
Motivation:

When writing an HTTP/2 HEADERS with END_STREAM=1, the application expects
the stream to be closed afterward. However, the write can fail locally
due to HPACK encoder and similar. When that happens we need to make sure
to issue a RST_STREAM otherwise the stream can be closed locally but
orphaned remotely. The RST_STREAM is typically handled by
Http2ConnectionHandler.onStreamError, which will only send a RST_STREAM
if that stream still exists locally.

There are two possible flows for trailers, one handled immediately and
one going through the flow controller. Previously they behaved
differently, with the immedate code calling the error handler after
closing the stream. The immediate code also used a listener for calling
closeStreamLocal while the flow controlled code did so immediately after
the write.

The two code paths also differed in their VoidChannelPromise handling,
but both were broken. The immediate code path called unvoid() only if
END_STREAM=1, however it could always potentially add a listener via
notifyLifecycleManagerOnError(). And the flow controlled code path
unvoided incorrectly, changing the promise completion behavior. It also
passed the wrong promise to closeStreamLocal() in FlowControlledBase.

Modifications:

Move closeStreamLocal handling after calls to onError. This is the
primary change.

Now call closeStreamLocal immediately instead of when the future
completes. This is the more likely correct behavior as it matches that
of DATA frames.

Fix all the VoidChannelPromise handling.

Result:

Http2ConnectionHandler.onStreamError sees the same state as the remote
and issues a RST_STREAM, properly cleaning up the stream.
2018-09-28 10:29:12 -07:00
Roger
6138541033 Avoid repeating the same field and hiding it (#8335)
Motivation

The EpollChannelConfig (same for KQueues) and its subclasses repeatetly declare their own channel field which leads to a 3x repetition for each config instance. Given the fields are protected or package-private it's exposing the code code to "field hiding" bugs.

Modifications

Use the the existing protected channel field from the DefaultChannelConfig class and simply cast it when needed.

Result

Fixes #8331
2018-09-28 17:37:14 +02:00
Norman Maurer
59973e93dd
Ensure X509KeyManager methods are called on the correct time when using server-side and support more methods of ExtendedSSLSession. (#8283)
Motivation:

Before when on server-side we just called the X509KeyManager methods when handshake() was called the first time which is not quite correct as we may not have received the full SSL hello / handshake and so could not extra for example the SNI hostname that was requested.
OpenSSL exposes the SSL_CTX_set_cert_cb function which allows to set a callback which is executed at the correct moment, so we should use it. This also allows us to support more methods of ExtendedSSLSession easily.

Modifications:

- Make use of new methods exposed by netty-tcnative since https://github.com/netty/netty-tcnative/pull/388 to ensure we select the key material at the correct time.
- Implement more methods of ExtendedOpenSslSession
- Add unit tests to ensure we are able to retrieve various things on server-side in the X509KeyManager and so verify it is called at the correct time.
- Simplify code by using new netty-tcnative methods.

Result:

More correct implementation for server-side usage and more complete implemented of ExtendedSSLSession.
2018-09-28 11:34:38 +02:00
Norman Maurer
73acac13f4
Check if hostname validation is supported before trying to use in test. (#8333)
Motivation:

a208f6dc7c added a testcase which uses hostname validation which may not be supported by OpenSSL depending on the version that is used. We should check first before we try to use it.

Modifications:

Add assumeTrue(...) check to ensure hostname validation is supported before trying to run the test.

Result:

No more test-failures on OpenSSL versions < 1.0.2.
2018-09-28 10:54:05 +02:00
Norman Maurer
a208f6dc7c
Do the same extended checks as the JDK when a X509TrustManager is used with the OpenSSL provider. (#8307)
Motivation:

When a X509TrustManager is used while configure the SslContext the JDK automatically does some extra checks during validation of provided certs by the remote peer. We should do the same when our native implementation is used.

Modification:

- Automatically wrap a X509TrustManager and so do the same validations as the JDK does.
- Add unit tests.

Result:

More consistent behaviour. Fixes https://github.com/netty/netty/issues/6664.
2018-09-28 09:19:58 +02:00
root
2d7cb47edd [maven-release-plugin] prepare for next development iteration 2018-09-27 19:00:45 +00:00
root
3a9ac829d5 [maven-release-plugin] prepare release netty-4.1.30.Final 2018-09-27 18:56:12 +00:00
Norman Maurer
b81c8ed55c
Use AuthoritativeDnsServerCache for creating the new redirect stream. (#8316)
* Use AuthoritativeDnsServerCache for creating the new redirect stream.

Motivation:

At the moment if a user wants to provide custom sorting of the nameservers used for redirects it needs to be implemented in two places. This is more complicated as it needs to be.

Modifications:

- Just delegate to the AuthoritativeDnsServerCache always as we fill it before we call newRedirectDnsServerStream anyway.

Result:

Easier way for the user to implement custom sorting.
2018-09-27 19:45:58 +02:00
Norman Maurer
5650db5826
Add cache for CNAME mappings resolved during lookup of DNS entries. (#8314)
* Add cache for CNAME mappings resolved during lookup of DNS entries.

Motivation:

If the CNAMEd hostname is backed by load balancing component, typically the final A or AAAA DNS records have small TTL. However, the CNAME record itself is setup with longer TTL.

For example:
* x.netty.io could be CNAMEd to y.netty.io with TTL of 5 min
* A / AAAA records for y.netty.io has a TTL of 0.5 min

In current Netty implementation, original hostname is saved in resolved cached with the TTL of final A / AAAA records. When that cache entry expires, Netty recursive resolver sends at least two queries — 1st one to be resolved as CNAME record and the 2nd one to resolve the hostname in CNAME record.
If CNAME record was cached, only the 2nd query would be needed most of the time. 1st query would be needed less frequently.

Modifications:

Add a new CnameCache that will be used to cache CNAMEs and so may reduce queries.

Result:

Less queries needed when CNAME is used.
2018-09-27 17:05:35 +02:00
Norman Maurer
70efd25801
Add docker-compose config to run with Java12 (#8327)
Motivation:

The first EA builds for Java12 are released so we should allow to run with these in our docker-compose setup.

Modifications:

Add docker-compose configs for Java12.

Result:

Be able to run easily with Java12 as well.
2018-09-27 12:47:17 +02:00
Norman Maurer
652650e127
Fix leak in SniClientJava8TestUtil (#8326)
Motivation:

4d1458604a did fix some leaks in SniClientTest but missed the ones in SniClientJava8TestUtil.

Modifications:

Correctly release SslContext.

Result:

No more leaks in SNI tests.
2018-09-27 09:32:41 +02:00
Norman Maurer
9acd9273c1
Allow to run tests with leak detection enabled. (#8323)
Motivation:

We should add some command to be able to run all tests with leak detection enabled. This will then be used on the CI during PR builds.

Modifications:

Add new docker-compose config to run with leak-detection enabled.

Result:

Easy way to enable leak detection while running tests via docker.
2018-09-27 09:13:28 +02:00
Norman Maurer
c546ab20a1
Ensure ByteToMessageDecoder.Cumulator implementations always release in buffer. (#8325)
Motivation:

We need to ensure the Cumulator always releases the input buffer if it can not take over the ownership of it as otherwise it may leak.

Modifications:

- Correctly ensure the buffer is always released.
- Add unit tests.

Result:

Ensure buffer is always released.
2018-09-27 07:38:42 +02:00
Matt Ayres
ba594bcf4a Fixed illegal reflective access by not relying on a sun.net.dns class. (#8318) (#8319)
Motivation

Applications should not depend on internal packages with Java 9 and later. This cause a warning now, but will break in future versions of Java.

Modification

This change adds methods to UnixResolverDnsServerAddressStreamProvider (following after #6844) that parse /etc/resolv.conf for domain and search entries. Then DnsNameResolver does not need to rely on sun.net.dns.ResolverConfiguration to do this.

Result

Fixes #8318. Furthermore, at least in my testing with Java 11, this also makes multiple search entries work properly (previously I was only getting the first entry).
2018-09-26 20:55:46 +02:00
Norman Maurer
618a98fdb5
Add profile to be able to compile on java12 (#8321)
Motivation:

First EA releases of Java12 are out we should be able to compile with these and run tests.

Modifications:

Add maven profile for java12.

Result:

Be able to use Java12
2018-09-26 20:01:53 +02:00
Norman Maurer
4d1458604a
Fix leak in SniClientTest. (#8324)
Motivation:

We need to release the ReferenceCountedSslContext to eliminate resource leaks.

Reported in https://garage.netty.io/teamcity/viewLog.html?buildId=33353&buildTypeId=netty_build_oraclejdk8&tab=buildLog#_focus=157264.

Modifications:

Call release on the SslContext instances.

Result:

No more leaks in tests.
2018-09-26 19:59:00 +02:00
Norman Maurer
60a7ece4c3
Update to final Java11 release (#8320)
Motivation:

We should use final Java11 release during builds.

Modifications:

Update to final Java11 release

Result:

Use latest release.
2018-09-26 12:16:19 +02:00
Norman Maurer
9a3be347af
Ensure we always encode all data in JdkZlibEncoder. (#8305)
Motivation:

In theory our estimation of the needed buffer could be off and so we need to ensure we grow it if there is no space left.

Modifications:

Ensure we grow the buffer if there is no space left in there but we still have data to deflate.

Result:

Correctly deflate data in all cases.
2018-09-22 13:34:12 -07:00
Norman Maurer
a80c49828f
Cleanup SSL test. (#8301)
Motivation:

I noticed that we had some errors showing up in a test (which did not fail it tho) because we tried to full-fill the promise multiples times.

Modifications:

Use trySuccess(...) as we may produce multiple exceptions.

Result:

Less errors during test-run.
2018-09-21 08:04:24 -07:00
Norman Maurer
01db30a163
Correctly implement ExtendedSSLSession.getStatusResponses() for ReferenceCountedOpenSslEngine (#8297)
Motivation:

Java9 added getStatusResponses() to ExtendedSSLSession which we should correctly support when possible.

Modifications:

Implement the method correctly.

Result:

More complete and correct implementation.
2018-09-19 17:13:44 -07:00
Björn Kautler
1b6e47ab2b Fix incorrectly encoded empty SOCKS5 address (#8292)
Motivation:

If you encode a SOCKS5 message like new DefaultSocks5CommandResponse(FAILURE, DOMAIN, "", 0)
you correctly get a result of 05010003000000.
But if the bndAddr is null, for example like new DefaultSocks5CommandResponse(FAILURE, DOMAIN)
the encoded result is 0501000301000000 which means the domain name has a length of one and consists of a 0-byte.

Modification:

With this commit it is also correctly encoded as a string of 0 length.

Result:

Correctly encode empty SOCKS5 address
2018-09-18 14:20:28 -07:00
Norman Maurer
687275361f
Update to Conscrypt 1.3.0 (#8296)
Motivation:

Conscrypt 1.3.0 was just released and adds support for TLSv1.3

Modifications:

Update to 1.3.0

Result:

Use latest conscrypt during build / test.
2018-09-18 14:19:06 -07:00
Andrey Mizurov
2ab3e13f08 Fix get charset from content-type header with multiple parameters (#8286)
Motivation:

Get charset from Content-Type header even it contains multiple parameters.

Modification:

Extract charset value from the charset parameter if it is not last.

Result:

Fixes #8273
2018-09-14 21:39:01 +02:00
Norman Maurer
34d52fcbfe
Implemented ExtendedOpenSslSession.getStatusResponses() so it not throws an UnsupportedOperationException. (#8290)
Motivation:

6ed7c6c75d added support for ExtendedOpenSslSession but we did not override getStatusResponses(). This lead to test failures on java9.

Modifications:

Implement ExtendedOpenSslSession.getStatusResponses() so it just returns an empty list.

Result:

Test pass again on Java9.
2018-09-14 20:33:09 +02:00
Norman Maurer
2b1514ec5a
Only use KeyManagerFactory in SniClientTest when supported by OpenSSL version. (#8289)
Motivation:

6ed7c6c75d added a test which blindly assumed we can use a KeyManagerFactory all the time. This is only true if have OpenSSL 1.0.2 or later, which may not be the case.

Modifications:

Only use KeyManagerFactory in test if the OpenSSL version does support it.

Result:

More robust tests.
2018-09-14 19:01:55 +02:00
Norman Maurer
6ed7c6c75d
Return an ExtendSSLSession whenever possible to allow more strict checking when using OpenSSL (#8281)
Motivation:

When an ExtendedSSLSession is used its possible to do more strict checking of the keys during handshake. We should do this whenever possible.

Modification:

- Return an ExtendedSSLSession when using client-mode and Java7+
- Add unit test
- Simplify unit tests

Result:

More consistent behaviour.
2018-09-14 14:33:11 +02:00
Norman Maurer
9eb124bb62
Don't cause ClassCastException if registration fails during constructing DnsNameResolver. (#8280)
Motivation:

We should not try to cast the Channel to a DatagramChannel as this will cause a ClassCastException.

Modifications:

- Do not cast
- rethrow from constructor if we detect the registration failed.
- Add unit test.

Result:

Propagate correct exception.
2018-09-11 20:34:37 +02:00
Carl Mastrangelo
1dff107de1 Don't re-arm timerfd each epoll_wait (#7816)
Motivation:
The Epoll transport checks to see if there are any scheduled tasks
before entering epoll_wait, and resets the timerfd just before.
This causes an extra syscall to timerfd_settime before doing any
actual work.   When scheduled tasks aren't added frequently, or
tasks are added with later deadlines, this is unnecessary.

Modification:
Check the *deadline* of the peeked task in EpollEventLoop, rather
than the *delay*.  If it hasn't changed since last time, don't
re-arm the timer

Result:
About 2us faster on gRPC RTT 50pct latency benchmarks.

Before (2 runs for 5 minutes, 1 minute of warmup):

```
50.0%ile Latency (in nanos):		64267
90.0%ile Latency (in nanos):		72851
95.0%ile Latency (in nanos):		78903
99.0%ile Latency (in nanos):		92327
99.9%ile Latency (in nanos):		119691
100.0%ile Latency (in nanos):		13347327
QPS:                           14933

50.0%ile Latency (in nanos):		63907
90.0%ile Latency (in nanos):		73055
95.0%ile Latency (in nanos):		79443
99.0%ile Latency (in nanos):		93739
99.9%ile Latency (in nanos):		123583
100.0%ile Latency (in nanos):		14028287
QPS:                           14936
```

After:
```
50.0%ile Latency (in nanos):		62123
90.0%ile Latency (in nanos):		70795
95.0%ile Latency (in nanos):		76895
99.0%ile Latency (in nanos):		90887
99.9%ile Latency (in nanos):		117819
100.0%ile Latency (in nanos):		14126591
QPS:                           15387

50.0%ile Latency (in nanos):		61021
90.0%ile Latency (in nanos):		70311
95.0%ile Latency (in nanos):		76687
99.0%ile Latency (in nanos):		90887
99.9%ile Latency (in nanos):		119527
100.0%ile Latency (in nanos):		6351615
QPS:                           15571
```
2018-09-11 13:38:38 +02:00
Matteo Merli
2a1596a4e9 Allow to configure socket option SO_BUSY_POLL (#8268)
Motivation:

When using Epoll based transport, allow applications to configure SO_BUSY_POLL socket option:

       SO_BUSY_POLL (since Linux 3.11)
              Sets the approximate time in microseconds to busy poll on a
              blocking receive when there is no data.  Increasing this value
              requires CAP_NET_ADMIN.  The default for this option is con‐
              trolled by the /proc/sys/net/core/busy_read file.

              The value in the /proc/sys/net/core/busy_poll file determines
              how long select(2) and poll(2) will busy poll when they oper‐
              ate on sockets with SO_BUSY_POLL set and no events to report
              are found.

              In both cases, busy polling will only be done when the socket
              last received data from a network device that supports this
              option.

              While busy polling may improve latency of some applications,
              care must be taken when using it since this will increase both
              CPU utilization and power usage.

Modification:

Added SO_BUSY_POLL socket option
Result:

Able to configure SO_BUSY_POLL from Netty
2018-09-07 20:50:51 +02:00
Norman Maurer
c14efd952d
Directly init refCnt to 1 (#8274)
Motivation:

We should just directly init the refCnt to 1 and not use the AtomicIntegerFieldUpdater.

Modifications:

Just assing directly to 1.

Result:

Cleaner code and possible a bit faster as the JVM / JIT may be able to optimize the first store easily.
2018-09-07 19:04:19 +02:00
Norman Maurer
e542a2cf26
Use a non-volatile read for ensureAccessible() whenever possible to reduce overhead and allow better inlining. (#8266)
Motiviation:

At the moment whenever ensureAccessible() is called in our ByteBuf implementations (which is basically on each operation) we will do a volatile read. That per-se is not such a bad thing but the problem here is that it will also reduce the the optimizations that the compiler / jit can do. For example as these are volatile it can not eliminate multiple loads of it when inline the methods of ByteBuf which happens quite frequently because most of them a quite small and very hot. That is especially true for all the methods that act on primitives.

It gets even worse as people often call a lot of these after each other in the same method or even use method chaining here.

The idea of the change is basically just ue a non-volatile read for the ensureAccessible() check as its a best-effort implementation to detect acting on already released buffers anyway as even with a volatile read it could happen that the user will release it in another thread before we actual access the buffer after the reference check.

Modifications:

- Try to do a non-volatile read using sun.misc.Unsafe if we can use it.
- Add a benchmark

Result:

Big performance win when multiple ByteBuf methods are called from a method.

With the change:
UnsafeByteBufBenchmark.setGetLongUnsafeByteBuf  thrpt   20  281395842,128 ± 5050792,296  ops/s

Before the change:
UnsafeByteBufBenchmark.setGetLongUnsafeByteBuf  thrpt   20  217419832,801 ± 5080579,030  ops/s
2018-09-07 07:47:02 +02:00
Norman Maurer
afe0767e9c
Log the correct line-number when using SLF4j with netty if possible. (#8258)
* Log the correct line-number when using SLF4j with netty if possible.

Motivation:

At the moment we do not log the correct line number in many cases as it will log the line number of the logger wrapper itself. Slf4j does have an extra interface that can be used to filter out these nad make it more usable with logging wrappers.

Modifications:

Detect if the returned logger implements LocationAwareLogger and if so make use of its extra methods to be able to log the correct origin of the log request.

Result:

Better logging when using slf4j.
2018-09-07 07:34:22 +02:00
Norman Maurer
052c2fbefe
Update to jmh 1.2.1 (#8270)
Motivation:

We should use the latest jmh version which also supports -prof dtraceasm on MacOS.

Modifications:

Update to latest jmh version.

Result:

Better benchmark / profiling support on MacOS.
2018-09-06 22:31:52 +02:00