-
Notifications
You must be signed in to change notification settings - Fork 542
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SUREFIRE-1846] Remove Base64 in the Encoder/Decoder and gain the performance for the communication flow: Fork to Plugin #310
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good!
I left a few nits
private static final int BUFFER_SIZE = 1024; | ||
private static final byte[] MAGIC_NUMBER_BYTES = MAGIC_NUMBER.getBytes( US_ASCII ); | ||
private static final byte[] STREAM_ENCODING_BYTES = STREAM_ENCODING.name().getBytes( US_ASCII ); | ||
private static final int DELIMINATOR_LENGTH = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo DELIMITER_LENGTH
@Test | ||
public void test5() throws IOException, InterruptedException | ||
{ | ||
/*final CharsetDecoder decoder = STREAM_ENCODING.newDecoder() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it commented intentionally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah no, these tests yet contain performance tests. Now they don't have a value for us and they can be deleted.
Yet only EventConsumerThread
can be reviewed.
Initially i used the tests to measure several approaches for decoding byte[] to String, e.g. new String()
vs CharacterBuffer, strings concatenation, decoding with/out number-of-bytes in the stream :2:AB:
, i had to measure time of objects creation, measured the exec time of HashMap and hashcode/equals methods - see the class Segment
and measured the performance of binary data tree against hashcode/equals. So initially these tests were used for selecting the right approach means performance/code-approach.
{ | ||
StringBuilder builder = new StringBuilder( 256 ); | ||
TimeUnit.SECONDS.sleep( 2 ); | ||
System.gc(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was deleted.
Pls have a look at the commits. There are improvements in EventConsumerThread
.
I have added the tests in EventConsumerThread
with 47% coverage. I continue with testing it more...
…formance for the communication flow: Fork to Plugin
@eolivelli |
Sure +1 |
Done. |
No description provided.