Commit Graph

605 Commits

Author SHA1 Message Date
Dmitriy Dumanskiy
42376c052a Java 8 migration. Inline PlatformDependent.newConcurrentHashMap() (#8760)
Motivation:

PlatformDependent.newConcurrentHashMap() is no longer needed so it could be easily removed and new ConcurrentHashMap<>() inlined instead of invoking PlatformDependent.newConcurrentHashMap().

Modification:

Use ConcurrentHashMap provided by the JDK directly.

Result:

Less code to maintain.
2019-01-22 17:18:50 +01:00
田欧
9d62deeb6f Java 8 migration: Use diamond operator (#8749)
Motivation:

We can use the diamond operator these days.

Modification:

Use diamond operator whenever possible.

Result:

More modern code and less boiler-plate.
2019-01-22 16:07:26 +01:00
Dmitriy Dumanskiy
c34340fff8 Java 8 migration. Auto close for try catch blocks (#8752)
Motivation:

Since Java 7 we can automatically close resources in try () construction.

Modification:

Changed all try catches in the code with autoclose try (resource)

Result:

Less boiler-plate
2019-01-22 15:57:30 +01:00
Norman Maurer
4a82d107d2
Require Java8 as minimum (#8739)
Motivation:

While we are not yet quite sure if we want to require Java11 as minimum we are at least sure we want to use java8 as minimum.

Modifications:

Change minimum version to java8 and update some tests which failed compilation after this change.

Result:

Use Java8 as minimum and be able to use Java8 features.
2019-01-22 08:48:18 +01:00
kashike
c0aa1ea5c7 Fix minor spelling issues in javadocs (#8701)
Motivation:

Javadocs contained some spelling errors, we should fix these.

Modification:

Fix spelling

Result:

Javadoc cleanup.
2019-01-14 07:25:13 +01:00
Alex Vasiliev
37e471dbe6 Added comments to LineBasedFrameDecoder, JsonObjectDecoder and XmlFrameDecoder that they are only compatible with UTF-8 encoded streams. (#8651)
Motivation:

LineBasedFrameDecoder, JsonObjectDecoder and XmlFrameDecoder upon investigation of the
sourcecode appeared to only support ASCII or UTF-8 input. It is an important characteristic
and ont reflected in any documentation. This could lead to improper usage and bugs.

Modifications:

Javadoc comment is addedd to all three classes to state that implementation is only
compatible with UTF-8 or ASCII input streams and brifly touches on implementaion details.

Result:

The end user of the netty library would not have to study sorcecode to deterime character
encoding limitations for given classes.
2018-12-20 07:40:29 +01:00
Norman Maurer
d9a6cf341c
Remove support for marking reader and writerIndex in ByteBuf to reduce overhead and complexity. (#8636)
Motivation:

ByteBuf supports “marker indexes”. The intended use case for these is if a speculative operation (e.g. decode) is in process the user can “mark” and interface and refer to it later if the operation isn’t successful (e.g. not enough data). However this is rarely used in practice,
requires extra memory to maintain, and introduces complexity in the state management for derived/pooled buffer initialization, resizing, and other operations which may modify reader/writer indexes.

Modifications:

Remove support for marking and adjust testcases / code.

Result:

Fixes https://github.com/netty/netty/issues/8535.
2018-12-11 14:00:49 +01:00
Norman Maurer
0cc13cb0d4 Fix typo in MessageToMessageDecoder api docs. (#8638)
Motivation:

We had some typo (most likely caused by copy-and-paste) in the api docs which should be fixed.

Modifications:

Replace encoder by decoder word.

Result:

Correct apidocs.
2018-12-07 20:45:36 +01:00
Bryce Anderson
6563f23a9b Don't swallow intermediate write failures in MessageToMessageEncoder (#8454)
Motivation:

If the encoder needs to flush more than one outbound message it will
create a new ChannelPromise for all but the last write which will
swallow failures.

Modification:

Use a PromiseCombiner in the case of multiple messages and the parent
promise isn't the `VoidPromise`.

Result:

Intermediate failures are propagated to the original ChannelPromise.
2018-11-03 10:36:26 +01:00
Nick Hill
d7fa7be67f Exploit PlatformDependent.allocateUninitializedArray() in more places (#8393)
Motivation:

There are currently many more places where this could be used which were
possibly not considered when the method was added.

If https://github.com/netty/netty/pull/8388 is included in its current
form, a number of these places could additionally make use of the same
BYTE_ARRAYS threadlocal.

There's also a couple of adjacent places where an optimistically-pooled
heap buffer is used for temp byte storage which could use the
threadlocal too in preference to allocating a temp heap bytebuf wrapper.
For example
https://github.com/netty/netty/blob/4.1/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java#L1417.

Modifications:

Replace new byte[] with PlatformDependent.allocateUninitializedArray()
where appropriate; make use of ByteBufUtil.getBytes() in some places
which currently perform the equivalent logic, including avoiding copy of
backing array if possible (although would be rare).

Result:

Further potential speed-up with java9+ and appropriate compile flags.
Many of these places could be on latency-sensitive code paths.
2018-10-27 10:43:28 -05:00
Nick Hill
583d838f7c Optimize AbstractByteBuf.getCharSequence() in US_ASCII case (#8392)
* Optimize AbstractByteBuf.getCharSequence() in US_ASCII case

Motivation:

Inspired by https://github.com/netty/netty/pull/8388, I noticed this
simple optimization to avoid char[] allocation (also suggested in a TODO
here).

Modifications:

Return an AsciiString from AbstractByteBuf.getCharSequence() if
requested charset is US_ASCII or ISO_8859_1 (latter thanks to
@Scottmitch's suggestion). Also tweak unit tests not to require Strings
and include a new benchmark to demonstrate the speedup.

Result:

Speed-up of AbstractByteBuf.getCharSequence() in ascii and iso 8859/1
cases
2018-10-26 15:32:38 -07: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
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
dc1b511fcf
Correctly reset offset when fail lazy because of too long frame. (#8257)
Motivation:

We need to reset the offset to 0 when we fail lazy because of a too long frame.

Modifications:

- Reset offset
- Add testcase

Result:

Fixes https://github.com/netty/netty/issues/8256.
2018-09-04 19:13:56 +02:00
Norman Maurer
1611acf4ce
Fix CharSequenceValueConverter.convertToByte implementation for AsciiString (#7994)
Motivation:

The implementation of CharSequenceValueConverter.convertToByte did not correctly handle AsciiString if the length != 1.

Modifications:

- Only use fast-path for AsciiString with length of 1.
- Add unit tests.

Result:

Fixes https://github.com/netty/netty/issues/7990
2018-06-01 21:15:08 +02:00
Norman Maurer
1d09efeeb2
Correctly copy existing elements when CodecOutputList.add(index, element) is called. (#7939)
Motivation:

We did not correctly copy elements in some cases when add(index, element) was used.

Modifications:

- Correctly detect when copy is neede and when not.
- Add test case.

Result:

Fixes https://github.com/netty/netty/issues/7938.
2018-05-15 19:41:45 +02:00
Nikolay Fedorovskikh
401b196623 Extract common parts from if statements (#7831)
Motivation:
Some `if` statements contains common parts that can be extracted.

Modifications:
Extract common parts from `if` statements.

Result:
Less code and bytecode. The code is simpler and more clear.
2018-04-11 14:36:56 +02:00
ikurovsky
c58069f284 Better handling of streaming JSON data in JsonObjectDecoder (#7821)
Motivation:

When the JsonObjectDecoder determines that the incoming buffer had some data discarded, it resets the internal index to readerIndex and attempts to adjust the state which does not correctly work for streams of JSON objects.

Modifications:

Reset the internal index to the value considering the previous reads.

Result:

JsonObjectDecoder correctly handles streams of both JSON objects and arrays with no state adjustments or repeatable reads.
2018-04-05 07:58:14 +02:00
Norman Maurer
6e6cfa0604 Add tests for EmptyHeaders
Motivation:

6e5fd9311f fixed a bug in EmptyHeaders which was never noticed before because we had no tests.

Modifications:

Add tests for EmptyHeaders.

Result:

EmptyHeaders is tested now.
2018-03-20 15:59:08 +01:00
Scott Mitchell
6e5fd9311f EmptyHeaders get with default value returns null
Motivation:
EmptyHeaders#get with a default value argument returns null. It should never return null, and instead it should return the default value.

Modifications:
- EmptyHeaders#get with a default value should return that default value

Result:
More correct implementation of the Headers API.
2018-03-19 17:54:26 +01:00
David Nault
f40ecc3f10 Fix Snappy decoding of large 2-byte literal lengths and copy offsets
Motivation:

The Snappy decoder was failing on valid inputs containing literals
with 2-byte lengths > 0x8000 or copies with 2-byte offsets >= 0x8000.

The decoder was also enforcing an artificially low offset limit of
0x7FFF, something the Snappy format description advises against,
and which prevents decoding valid inputs generated by other encoders.

Modifications:

Interpret 2-byte literal lengths and 2-byte copy offsets as unsigned
shorts, in accordance with the format description and reference
implementation.

Allow any positive offset value. Throw an appropriate exception
for negative values (which can theoretically occur due to arithmetic
overflow on 4-byte offsets, but are unlikely to occur in the wild).

Result:

The Snappy decoder can handle valid inputs that previously caused
it to throw exceptions.
2018-02-20 11:42:23 +01:00
Scott Mitchell
1e5fafe446 Simplify CharSequenceValueConverter#convertToBoolean
Motivation:
CharSequenceValueConverter#convertToBoolean has a few manual conditionals which can be removed if we use AsciiString.contentEqualsIgnoreCase. Also by comparing an AsciiString to a String we will incur conversions to char that can be avoided if we compare against AsciiString.

Modifications:
- Use AsciiString.contentEqualsIgnoreCase
- Compare against a AsciiString

Result:
Simplified CharSequenceValueConverter#convertToBoolean which favors AsciiString comparison.
2018-02-17 07:47:44 +01:00
Norman Maurer
756854e99a Correctly implement CharSequenceValueConvert.convertTimeMillis
Motivation:

If you pass the output of CharSequenceValueConvert.convertToTimeMillis to convertTimeMillis it will throw a ParseException.

Modifications:

- Correctly implement CharSequenceValueConverter.convertTimeMillis
- Add unit-tests for CharSequenceValueConverter

Result:

Correctly convert timemillis.
2018-02-16 07:44:13 +01:00
ryu1-sakai
c1d0d88f0a Implement DefaultHeaders.HeaderEntry.equals()
Motivation:

HeaderEntry.equals() inherets Object.equals() which simply check if two objects are the same.
So it returns false even when two HeaderEntry objects have the same name and value.

Modifications:

Implement HeaderEntry.equals() that follows the specification of Map.Entry.equals().
https://docs.oracle.com/javase/9/docs/api/java/util/Map.Entry.html#equals-java.lang.Object-

Result:

HeaderEntry.equals() returns true if two HeaderEntry objects have the same name and value.
2018-02-15 13:07:03 +01:00
Norman Maurer
ad6af3250c DefaultHeaders / CharSequenceValueConverter should treat boolean consistently.
Motivation:

HttpHeaders.getBoolean should return the same truth value for the same string value, regardless of the underlying type.

Modifications:

- Only treat values of true as Boolean.TRUE
- Add unit tests.

Result:

Consistent converting of values for all CharSequence implementations.
2018-02-15 08:37:46 +01:00
Shohei Kamimori
73f23c5faa Fix typos in docs.
Motivation:

There are same typos in the docs.

Modifications:

Fix typos. Docs only changing.

Result:

More correct docs.
2018-02-14 08:44:07 +01:00
Norman Maurer
02b7507a62 Correctly handle the case when converting of value fails and return null or default value.
Motivation:

Headers.get* methods should not throw an exception but return null or the default value if converting of the value fails.

Modifications:

- Correctly handle the case when ValueConverter throws an Exception.
- Add testcase.

Result:

Fixes [#7710].
2018-02-14 08:35:55 +01:00
Norman Maurer
4c1e0f596a Use FastThreadLocal for CodecOutputList
Motivation:

We used Recycler for the CodecOutputList which is not optimized for the use-case of access only from the same Thread all the time.

Modifications:

- Use FastThreadLocal for CodecOutputList
- Add benchmark

Result:

Less overhead in our codecs.
2018-01-23 11:34:28 +01:00
Dmitriy Dumanskiy
e6c9ac968d Cleanup: replaced deprecated ctx.attr() and ctx.hasAttr() methods usage with ch.attr() and ch.hasAttr().
Motivation:

Will allow easy removal of deprecated methods in future.

Modification:

Replaced ctx.attr(), ctx.hasAttr() with ctx.channel().attr(), ctx.channel().hasAttr().

Result:

No deprecated ctx.attr(), ctx.hasAttr() methods usage.
2018-01-18 15:00:41 +00:00
Abhijit Sarkar
6ff48dcbe3 Fixes #7566 by handling concatenated GZIP streams.
Motivation:
According to RFC 1952, concatenation of valid gzip streams is also a valid gzip stream. JdkZlibDecoder only processed the first and discarded the rest.

Modifications:
- Introduced a constructor argument decompressConcatenated that if true, JdkZlibDecoder would continue to process the stream.

Result:
- If 'decompressConcatenated = true', concatenated streams would be processed in
compliance to RFC 1952.
- If 'decompressConcatenated = false' (default), existing behavior would remain.
2018-01-17 06:10:56 +01:00
Nikolay Fedorovskikh
c9668ce40f The constants calculation in compile-time
Motivation:
Allow pre-computing calculation of the constants for compiler where it could be.
Similar fix in OpenJDK: [1].

Modifications:
- Use parentheses.
- Simplify static initialization of `BYTE2HEX_*` arrays in `StringUtil`.

Result:
Less bytecode, possible faster calculations at runtime.

[1] https://bugs.openjdk.java.net/browse/JDK-4477961
2017-12-21 07:41:38 +01:00
Norman Maurer
942b993f2b Only enable validation of headers if original headers were validating as well.
Motiviation:

In our replace(...) methods we always used validation for the newly created headers while the original headers may not use validation at all.

Modifications:

- Only use validation if the original headers used validation as well.
- Ensure we create a copy of the headers in replace(...).

Result:

Fixes [#5226]
2017-12-21 07:32:29 +01:00
Norman Maurer
7aca99f986 DefaultHttpHeader.names().toArray(...) may throw ArrayStoreException
Motivation:

DefaultHttpHeader.names() exposes HTTP header names as a Set<String>. Converting the resulting set to an array using toArray(String[]) throws an exception: java.lang.ArrayStoreException: io.netty.util.AsciiString.

Modifications:

- Remove our custom implementation of toArray(...) (and others) by just extending AbstractCollection.
- Add unit test

Result:

Fixes [#7428].
2017-11-22 19:04:55 +01:00
Stephane Landelle
e420f857fa Optimize DefaultHeaders#toString and implement HttpHeaders#toString
Motivation:

For debugging/logging purpose, it would be convenient to have
HttpHeaders#toString implemented.

DefaultHeaders does implement toString be the implementation is suboptimal and allocates a Set for the names and Lists for values.

Modification:

* Introduce HeadersUtil#toString that provides a convenient optimized helper to implement toString for various headers implementations
* Have DefaultHeaders#toString and HttpHeaders#toString delegate their toString implementation to HeadersUtil

Result:

Convenient HttpHeaders#toString. Optimized DefaultHeaders#toString.
2017-11-22 08:42:59 +01:00
Scott Mitchell
93130b172a HttpObjectEncoder and MessageAggregator EMPTY_BUFFER usage
Motivation:
HttpObjectEncoder and MessageAggregator treat buffers that are not readable special. If a buffer is not readable, then an EMPTY_BUFFER is written and the actual buffer is ignored. If the buffer has already been released then this will not be correct as the promise will be completed, but in reality the original content shouldn't have resulted in any write because it was invalid.

Modifications:
- HttpObjectEncoder should retain/write the original buffer instead of using EMPTY_BUFFER
- MessageAggregator should retain/write the original ByteBufHolder instead of using EMPTY_BUFFER

Result:
Invalid write operations which happen to not be readable correctly reflect failed status in the promise, and do not result in any writes to the channel.
2017-11-03 07:03:19 +01:00
Idel Pivnitskiy
d93c607f93 Update links for actual Protobuf repo and documentation
Motivation:

Use actual links to new locations of Protobuf repo and documentation to
avoid problems when redirect will not work.

Modification:

Links in comments and all/pom.xml

Result:

Correct links to Protobuf resources
2017-10-23 07:41:35 +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
Idel Pivnitskiy
558097449c Add missed 'serialVersionUID' field for Serializable classes
Motivation:

Without a 'serialVersionUID' field, any change to a class will make
previously serialized versions unreadable.

Modifications:

Add missed 'serialVersionUID' field for all Serializable
classes.

Result:

Proper deserialization of previously serialized objects.
2017-10-21 14:41:18 +02:00
Jason Tedor
3fe1f71511 Do not treat errors as decoder exception (redux)
Motivation: Today when Netty encounters a general error while decoding
it treats this as a decoder exception. However, for fatal causes this
should not be treated as such, instead the fatal error should be carried
up the stack without the callee having to unwind causes. This was
probably done for byte to byte message decoder but is now done for all
decoders.

Modifications: Instead of translating any error to a decoder exception,
we let those unwind out the stack (note that finally blocks still
execute) except in places where an event needs to fire where we fire
with the error instead of wrapping in a decoder exception.

Result: Fatal errors will not be treated as innocent decoder exceptions.
2017-10-07 18:17:00 +02:00
Jason Tedor
5eca326c35 Do not treat errors as decoder exception
Motivation: Today when Netty encounters a general error while decoding
it treats this as a decoder exception. However, for fatal causes this
should not be treated as such, instead the fatal error should be carried
up the stack without the callee having to unwind causes.

Modifications: Instead of translating any error to a decoder exception,
we let those unwind out the stack (note that finally blocks still
execute).

Result: Fatal errors will not be treated as innocent decoder exceptions.
2017-10-05 08:26:42 +02:00
Jackie.Meng
80b8a91b70 Use offset finding eol avoid repeated scaning.
Motivation:

A large frame will be componsed by many packages. Every time the package
arrived, findEndOfLine will be called from the start of the buffer. It
will cause the complexity of reading frame equal to  O(n^2). This can be
eliminated by using a offset to mark the last scan position, when new
package arrived, just find the delimter from the mark. The complexity
will be O(n).

Modification:

Add a offset to mark the last scan position.

Result:

Better performance for read large frame.
2017-09-17 09:17:38 -07:00
Scott Mitchell
44bb3b6f3a DefaultHeaders value iterator
Motivation:
The Headers interface supports an interface to get all the headers values corresponding to a particular name. This API returns a List which requires intermediate storage and increases GC pressure.

Modifications:
- Add a method which returns an iterator over all the values for a specific name

Result:
Ability to iterator over values for a specific name with no intermediate collection.
2017-09-16 16:46:19 -07:00
Francesco Nigro
6780183a89 Makes LengthFieldBasedFrameDecoder::decode inlineable
Motivation:

The decode method is too large to be inlined with default compiler settings, hence the uncommon paths need to be packed and moved away form the common one.

Modifications:

The uncommon paths of the decode call (eg failures with thrown exceptions) are packed and moved in private methods in order to reduce the size of the common one
and let it being inlined.

Result:

The decode method is being inlined if the stack depth allows it.
2017-08-28 09:08:45 +02:00
Derek Perez
b18a201d02 various errorprone fixes.
Motivation:

Continuing to make netty happy when compiling through errorprone.

Modification:

Mostly comments, some minor switch statement changes.

Result:

No more compiler errors!
2017-08-23 12:49:58 +02:00
Carl Mastrangelo
03d89c2222 Use Constructor for reflective class instantiation.
Motivation:
Calling `newInstance()` on a Class object can bypass compile time
checked Exception propagation.  This is noted in Java Puzzlers,
as well as in ErrorProne:
http://errorprone.info/bugpattern/ClassNewInstance

Modifications:
Use the niladic constructor to create a new instance.

Result:
Compile time safety for checked exceptions
2017-08-18 09:42:36 +02:00
Norman Maurer
123e07ca80 Revert "Only call ctx.fireChannelReadComplete() if ByteToMessageDecoder decoded at least one message."
This reverts commit d63bb4811e as this not covered correctly all cases and so could lead to missing fireChannelReadComplete() calls. We will re-evalute d63bb4811e and resbumit a pr once we are sure all is handled correctly
2017-08-18 09:06:37 +02:00
Violeta Georgieva
db4781282f Handle partially decoded elements while streaming Json array
Motivation:

'insideString' and 'openBraces' need a proper handling when streaming
Json array over multiple writes and an element decoding was started but
not completed.
Related to #6969

Modifications:

If the idx is reset:
- 'insideString' has to be reset to 'false' in order to indicate that
  array element will be decoded from the beginning
- 'openBraces' has to be reset to '1' to indicate that Json array
  decoding is in progress.

Result:
Json array is properly decoded when in streaming mode
2017-08-08 08:48:01 +02:00
Norman Maurer
d63bb4811e Only call ctx.fireChannelReadComplete() if ByteToMessageDecoder decoded at least one message.
Motivation:

Its wasteful and also confusing that channelReadComplete() is called even if there was no message forwarded to the next handler.

Modifications:

- Only call ctx.fireChannelReadComplete() if at least one message was decoded
- Add unit test

Result:

Less confusing behavior. Fixes [#4312].
2017-08-04 10:54:56 +02:00
Nikolay Fedorovskikh
6ab9c177ac Fix hash function and hash table size in Snappy
Motivation:

1. Hash function in the Snappy encoding is wrong probably: used '+' instead of '*'. See the reference implementation [1].
2. Size of the hash table is calculated, but not applied.

Modifications:

1. Fix hash function: replace addition by multiplication.
2. Allocate hash table with calculated size.
3. Use an `Integer.numberOfLeadingZeros` trick for calculate log2.
4. Release buffers in tests.

Result:

1. Better compression. In the test `encodeAndDecodeLongTextUsesCopy` now compressed size is 175 instead of 180 before this change.
2. No redundant allocations for hash table.
3. A bit faster the calc of shift (less an expensive math operations).

[1] 513df5fb5a/snappy.cc (L67)
2017-08-01 07:08:54 +02:00
Nikolay Fedorovskikh
068e64dbcf Fix a potential NPE 2017-07-31 20:39:20 +02:00