[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335111#comment-16335111
 ] 

ASF subversion and git services commented on ARTEMIS-1616:
--

Commit c6b6dd95d1665230d667557df240d8a62a2118af in activemq-artemis's branch 
refs/heads/master from [~nigro@gmail.com]
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=c6b6dd9 ]

ARTEMIS-1616 OpenWire improvements

Refactored OpenWireMessageConverter::toAMQMessage into smaller methods


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335108#comment-16335108
 ] 

ASF subversion and git services commented on ARTEMIS-1616:
--

Commit 64724c3520586c6cd1bc0aec9942ae5bb5562459 in activemq-artemis's branch 
refs/heads/master from [~nigro@gmail.com]
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=64724c3 ]

ARTEMIS-1616 OpenWire improvements

Used SimpleString on OpenWireMessageConverter to avoid translations on 
CoreMessage


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335110#comment-16335110
 ] 

ASF subversion and git services commented on ARTEMIS-1616:
--

Commit e7a1dca7b54bc47002b5d743115c1c790ead9579 in activemq-artemis's branch 
refs/heads/master from [~nigro@gmail.com]
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=e7a1dca ]

ARTEMIS-1616 OpenWire improvements

Used SimpleString on AMQSession with HDR_DUPLICATE_DETECTION_ID and 
CONNECTION_ID_PROPERTY_NAME


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335107#comment-16335107
 ] 

ASF subversion and git services commented on ARTEMIS-1616:
--

Commit 17c0a331aceda16460cc4d87f376c8730e479fe5 in activemq-artemis's branch 
refs/heads/master from [~nigro@gmail.com]
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=17c0a33 ]

ARTEMIS-1616 OpenWire improvements

Added existing queues cache to avoid multiple expensive 
AMQSession::checkAutoCreateQueue calls


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335105#comment-16335105
 ] 

ASF subversion and git services commented on ARTEMIS-1616:
--

Commit 54d0161850bc4543839d4f36b46f3bd84f5cd8e1 in activemq-artemis's branch 
refs/heads/master from [~nigro@gmail.com]
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=54d0161 ]

ARTEMIS-1616 OpenWire improvements

Refactored OpenWireMessageConverter::inbound into smaller methods


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335113#comment-16335113
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1786


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335112#comment-16335112
 ] 

ASF subversion and git services commented on ARTEMIS-1616:
--

Commit 0387b1a8427673f42166df5651bc504d38f365ce in activemq-artemis's branch 
refs/heads/master from [~nigro@gmail.com]
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=0387b1a ]

ARTEMIS-1616 OpenWire improvements

Refactored OpenWireMessageConverter::inbound into a private static method


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335104#comment-16335104
 ] 

ASF subversion and git services commented on ARTEMIS-1616:
--

Commit 2db4eafc4d81cd18f9394e356ee11b202f5f2301 in activemq-artemis's branch 
refs/heads/master from [~nigro@gmail.com]
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=2db4eaf ]

ARTEMIS-1616 OpenWire improvements

Avoided copy of CoreMessage when not needed and cached lambda on hot path


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16335106#comment-16335106
 ] 

ASF subversion and git services commented on ARTEMIS-1616:
--

Commit 051a3cae49c488314b9a5ccaa36056a358e4b9df in activemq-artemis's branch 
refs/heads/master from [~nigro@gmail.com]
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=051a3ca ]

ARTEMIS-1616 OpenWire improvements

Optimized SimpleString::split because heavily used into AddressImpl::new


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334608#comment-16334608
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1786#discussion_r163012542
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
 ---
@@ -361,13 +371,68 @@ public int hashCode() {
   }
}
 
+   private static SimpleString[] splitWithCachedString(final SimpleString 
simpleString, final int delim) {
+  final String str = simpleString.str;
+  final byte[] data = simpleString.data;
+  final int length = str.length();
+  List all = null;
+  int index = 0;
+  while (index < length) {
+ final int delimIndex = str.indexOf(delim, index);
+ if (delimIndex == -1) {
+//just need to add the last one
+break;
+ } else {
+all = addSimpleStringPart(all, data, index, delimIndex);
+ }
+ index = delimIndex + 1;
+  }
+  if (all == null) {
+ return new SimpleString[]{simpleString};
+  } else {
+ // Adding the last one
+ all = addSimpleStringPart(all, data, index, length);
+ // Converting it to arrays
+ final SimpleString[] parts = new SimpleString[all.size()];
+ return all.toArray(parts);
+  }
+   }
+
+   private static List 
addSimpleStringPart(List all,
+ final byte[] data,
+ final int 
startIndex,
+ final int 
endIndex) {
+  final int expectedLength = endIndex - startIndex;
+  final SimpleString ss;
+  if (expectedLength == 0) {
+ ss = EMPTY;
+  } else {
+ //extract a byte[] copy from this
+ final int ssIndex = startIndex << 1;
+ final int delIndex = endIndex << 1;
+ final byte[] bytes = Arrays.copyOfRange(data, ssIndex, delIndex);
+ ss = new SimpleString(bytes);
+  }
+  // We will create the ArrayList lazily
+  if (all == null) {
+ // There will be at least 3 strings on this case (which is the 
actual common usecase)
--- End diff --

@franz1981 just thinking on this is it worth making the other array list 
also 3? Just so both is consistent, maybe make it a constant, so if in future 
it changes again, single change would affect both.


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16334584#comment-16334584
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/1786
  
@franz1981 nudge, only thing holding me from merging this is the number of 
commits.
I guess really should be a @clebertsuconic nudge if he's asked for this on 
purpose.


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16332334#comment-16332334
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1786
  
@michaelandrepearce TBH that's what I was thinking to do, but 
@clebertsuconic suggested that having all the commits but just 1 pr wasn't a 
bad thing at all: he has more experience than me and I trust him so I think 
there could be some very good reasons (like reverting just what could break 
something, if any?) to suggest me that...I will ask him better when he will be 
online anyway :+1: 


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16332329#comment-16332329
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/1786
  
@franz1981 I assume you’ll squash commits once your ready right?


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16332326#comment-16332326
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1786#discussion_r162637206
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
 ---
@@ -361,13 +371,68 @@ public int hashCode() {
   }
}
 
+   private static SimpleString[] splitWithCachedString(final SimpleString 
simpleString, final int delim) {
+  final String str = simpleString.str;
+  final byte[] data = simpleString.data;
+  final int length = str.length();
+  List all = null;
+  int index = 0;
+  while (index < length) {
+ final int delimIndex = str.indexOf(delim, index);
+ if (delimIndex == -1) {
+//just need to add the last one
+break;
+ } else {
+all = addSimpleStringPart(all, data, index, delimIndex);
+ }
+ index = delimIndex + 1;
+  }
+  if (all == null) {
+ return new SimpleString[]{simpleString};
+  } else {
+ // Adding the last one
+ all = addSimpleStringPart(all, data, index, length);
+ // Converting it to arrays
+ final SimpleString[] parts = new SimpleString[all.size()];
+ return all.toArray(parts);
+  }
+   }
+
+   private static List 
addSimpleStringPart(List all,
+ final byte[] data,
+ final int 
startIndex,
+ final int 
endIndex) {
+  final int expectedLength = endIndex - startIndex;
+  final SimpleString ss;
+  if (expectedLength == 0) {
+ ss = EMPTY;
+  } else {
+ //extract a byte[] copy from this
+ final int ssIndex = startIndex << 1;
+ final int delIndex = endIndex << 1;
+ final byte[] bytes = Arrays.copyOfRange(data, ssIndex, delIndex);
+ ss = new SimpleString(bytes);
+  }
+  // We will create the ArrayList lazily
+  if (all == null) {
+ // There will be at least 3 strings on this case (which is the 
actual common usecase)
--- End diff --

Just to be clear I’m happy with this, as a PR, eg if you get all the bits 
you wanted done then don’t wait on this. It shouldn’t hold a merge. 

It’s just a question/query that was there before. Just a note that it be 
good to note what they are at some point.


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16332322#comment-16332322
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1786#discussion_r162636748
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
 ---
@@ -361,13 +371,68 @@ public int hashCode() {
   }
}
 
+   private static SimpleString[] splitWithCachedString(final SimpleString 
simpleString, final int delim) {
--- End diff --

Ah :) nice, very subtle. 


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16332195#comment-16332195
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user franz1981 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1786#discussion_r162610245
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
 ---
@@ -361,13 +371,68 @@ public int hashCode() {
   }
}
 
+   private static SimpleString[] splitWithCachedString(final SimpleString 
simpleString, final int delim) {
+  final String str = simpleString.str;
+  final byte[] data = simpleString.data;
+  final int length = str.length();
+  List all = null;
+  int index = 0;
+  while (index < length) {
+ final int delimIndex = str.indexOf(delim, index);
+ if (delimIndex == -1) {
+//just need to add the last one
+break;
+ } else {
+all = addSimpleStringPart(all, data, index, delimIndex);
+ }
+ index = delimIndex + 1;
+  }
+  if (all == null) {
+ return new SimpleString[]{simpleString};
+  } else {
+ // Adding the last one
+ all = addSimpleStringPart(all, data, index, length);
+ // Converting it to arrays
+ final SimpleString[] parts = new SimpleString[all.size()];
+ return all.toArray(parts);
+  }
+   }
+
+   private static List 
addSimpleStringPart(List all,
+ final byte[] data,
+ final int 
startIndex,
+ final int 
endIndex) {
+  final int expectedLength = endIndex - startIndex;
+  final SimpleString ss;
+  if (expectedLength == 0) {
+ ss = EMPTY;
+  } else {
+ //extract a byte[] copy from this
+ final int ssIndex = startIndex << 1;
+ final int delIndex = endIndex << 1;
+ final byte[] bytes = Arrays.copyOfRange(data, ssIndex, delIndex);
+ ss = new SimpleString(bytes);
+  }
+  // We will create the ArrayList lazily
+  if (all == null) {
+ // There will be at least 3 strings on this case (which is the 
actual common usecase)
--- End diff --

Difficult to say: I've found split to be used on AddressImpl mainly and 
openwire uses at least 3 parts addresses, while the other protocols seems to 
not rely heavily on split.
Probably @clebert has a better knowledge of how he has chosen 2 instead, 
wdyt?


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16331376#comment-16331376
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1786#discussion_r162492929
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
 ---
@@ -361,13 +371,68 @@ public int hashCode() {
   }
}
 
+   private static SimpleString[] splitWithCachedString(final SimpleString 
simpleString, final int delim) {
+  final String str = simpleString.str;
+  final byte[] data = simpleString.data;
+  final int length = str.length();
+  List all = null;
+  int index = 0;
+  while (index < length) {
+ final int delimIndex = str.indexOf(delim, index);
+ if (delimIndex == -1) {
+//just need to add the last one
+break;
+ } else {
+all = addSimpleStringPart(all, data, index, delimIndex);
+ }
+ index = delimIndex + 1;
+  }
+  if (all == null) {
+ return new SimpleString[]{simpleString};
+  } else {
+ // Adding the last one
+ all = addSimpleStringPart(all, data, index, length);
+ // Converting it to arrays
+ final SimpleString[] parts = new SimpleString[all.size()];
+ return all.toArray(parts);
+  }
+   }
+
+   private static List 
addSimpleStringPart(List all,
+ final byte[] data,
+ final int 
startIndex,
+ final int 
endIndex) {
+  final int expectedLength = endIndex - startIndex;
+  final SimpleString ss;
+  if (expectedLength == 0) {
+ ss = EMPTY;
+  } else {
+ //extract a byte[] copy from this
+ final int ssIndex = startIndex << 1;
+ final int delIndex = endIndex << 1;
+ final byte[] bytes = Arrays.copyOfRange(data, ssIndex, delIndex);
+ ss = new SimpleString(bytes);
+  }
+  // We will create the ArrayList lazily
+  if (all == null) {
+ // There will be at least 3 strings on this case (which is the 
actual common usecase)
--- End diff --

as noted what are these 3 "magical" use cases, it be good to enumerate 
them. I'm still left wondering :)


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16330228#comment-16330228
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user franz1981 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1786#discussion_r162272711
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
 ---
@@ -361,13 +371,68 @@ public int hashCode() {
   }
}
 
+   private static SimpleString[] splitWithCachedString(final SimpleString 
simpleString, final int delim) {
--- End diff --

To avoid the translation into into each time because String::indexOf is 
using int 


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16330227#comment-16330227
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user franz1981 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1786#discussion_r162272578
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
 ---
@@ -361,13 +371,68 @@ public int hashCode() {
   }
}
 
+   private static SimpleString[] splitWithCachedString(final SimpleString 
simpleString, final int delim) {
+  final String str = simpleString.str;
+  final byte[] data = simpleString.data;
+  final int length = str.length();
+  List all = null;
+  int index = 0;
+  while (index < length) {
+ final int delimIndex = str.indexOf(delim, index);
+ if (delimIndex == -1) {
+//just need to add the last one
+break;
+ } else {
+all = addSimpleStringPart(all, data, index, delimIndex);
+ }
+ index = delimIndex + 1;
+  }
+  if (all == null) {
+ return new SimpleString[]{simpleString};
+  } else {
+ // Adding the last one
+ all = addSimpleStringPart(all, data, index, length);
+ // Converting it to arrays
+ final SimpleString[] parts = new SimpleString[all.size()];
+ return all.toArray(parts);
+  }
+   }
+
+   private static List 
addSimpleStringPart(List all,
+ final byte[] data,
+ final int 
startIndex,
+ final int 
endIndex) {
+  final int expectedLength = endIndex - startIndex;
+  final SimpleString ss;
+  if (expectedLength == 0) {
+ ss = EMPTY;
+  } else {
+ //extract a byte[] copy from this
+ final int ssIndex = startIndex << 1;
+ final int delIndex = endIndex << 1;
+ final byte[] bytes = Arrays.copyOfRange(data, ssIndex, delIndex);
+ ss = new SimpleString(bytes);
+  }
+  // We will create the ArrayList lazily
+  if (all == null) {
+ // There will be at least 3 strings on this case (which is the 
actual common usecase)
--- End diff --

Good point: the ArrayList is temporary and although it won't be allocated 
on the stack having 2 instead of 3 won't make a big difference considering that 
will die young. I've noticed that most use cases need at least 3 to avoid 
enlarging of capacity so I've changed it: I haven't changed the original 
version yet as you have noticed and I could do :)
2 or 3 are very "magical" numbers anyway :P


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16330189#comment-16330189
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1786#discussion_r162266546
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
 ---
@@ -361,13 +371,68 @@ public int hashCode() {
   }
}
 
+   private static SimpleString[] splitWithCachedString(final SimpleString 
simpleString, final int delim) {
--- End diff --

why int not char, for delim


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16330164#comment-16330164
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1786#discussion_r162262699
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
 ---
@@ -361,13 +371,68 @@ public int hashCode() {
   }
}
 
+   private static SimpleString[] splitWithCachedString(final SimpleString 
simpleString, final int delim) {
+  final String str = simpleString.str;
+  final byte[] data = simpleString.data;
+  final int length = str.length();
+  List all = null;
+  int index = 0;
+  while (index < length) {
+ final int delimIndex = str.indexOf(delim, index);
+ if (delimIndex == -1) {
+//just need to add the last one
+break;
+ } else {
+all = addSimpleStringPart(all, data, index, delimIndex);
+ }
+ index = delimIndex + 1;
+  }
+  if (all == null) {
+ return new SimpleString[]{simpleString};
+  } else {
+ // Adding the last one
+ all = addSimpleStringPart(all, data, index, length);
+ // Converting it to arrays
+ final SimpleString[] parts = new SimpleString[all.size()];
+ return all.toArray(parts);
+  }
+   }
+
+   private static List 
addSimpleStringPart(List all,
+ final byte[] data,
+ final int 
startIndex,
+ final int 
endIndex) {
+  final int expectedLength = endIndex - startIndex;
+  final SimpleString ss;
+  if (expectedLength == 0) {
+ ss = EMPTY;
+  } else {
+ //extract a byte[] copy from this
+ final int ssIndex = startIndex << 1;
+ final int delIndex = endIndex << 1;
+ final byte[] bytes = Arrays.copyOfRange(data, ssIndex, delIndex);
+ ss = new SimpleString(bytes);
+  }
+  // We will create the ArrayList lazily
+  if (all == null) {
+ // There will be at least 3 strings on this case (which is the 
actual common usecase)
--- End diff --

Is it worth iterating these cases? E.g I'm left wondering what this is 
about :)


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16330160#comment-16330160
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1786#discussion_r162262393
  
--- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/SimpleString.java
 ---
@@ -361,13 +371,68 @@ public int hashCode() {
   }
}
 
+   private static SimpleString[] splitWithCachedString(final SimpleString 
simpleString, final int delim) {
+  final String str = simpleString.str;
+  final byte[] data = simpleString.data;
+  final int length = str.length();
+  List all = null;
+  int index = 0;
+  while (index < length) {
+ final int delimIndex = str.indexOf(delim, index);
+ if (delimIndex == -1) {
+//just need to add the last one
+break;
+ } else {
+all = addSimpleStringPart(all, data, index, delimIndex);
+ }
+ index = delimIndex + 1;
+  }
+  if (all == null) {
+ return new SimpleString[]{simpleString};
+  } else {
+ // Adding the last one
+ all = addSimpleStringPart(all, data, index, length);
+ // Converting it to arrays
+ final SimpleString[] parts = new SimpleString[all.size()];
+ return all.toArray(parts);
+  }
+   }
+
+   private static List 
addSimpleStringPart(List all,
+ final byte[] data,
+ final int 
startIndex,
+ final int 
endIndex) {
+  final int expectedLength = endIndex - startIndex;
+  final SimpleString ss;
+  if (expectedLength == 0) {
+ ss = EMPTY;
--- End diff --

How we sure, that expectedLength 0 shouldn't equal null?


> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16329433#comment-16329433
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1786
  
Please do not merge it yet: I need to update all the commit messages adding 
the JIRA.
The current PR has already passed the same OpenWire tests of master with 
similar results, but the most interesting part is that it improves dramatically 
the performances of OpenWire: on my box a 1 producer 1 consumer non durable 
queue with master perform ~75 K msg/sec while with this PR ~98 K msg/sec.



> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (ARTEMIS-1616) OpenWire improvements

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16329423#comment-16329423
 ] 

ASF GitHub Bot commented on ARTEMIS-1616:
-

GitHub user franz1981 opened a pull request:

https://github.com/apache/activemq-artemis/pull/1786

ARTEMIS-1616 OpenWire improvements

It includes several improvements on OpenWire:
- Avoided copy of CoreMessage when not needed and cached lambda on hot path
- Refactored OpenWireMessageConverter::inbound in order to group help the 
JVM compile most used methods
- Optimized SimpleString::split because heavily used into AddressImpl::new
- Added existing queues cache to avoid multiple expensive 
AMQSession::checkAutoCreateQueue calls
- used SimpleString on OpenWireMessageConverter to avoid translations on 
CoreMessage
- cached Notification Destination check on AMQConsumer
- Used SimpleString on AMQSession explicitly with 
HDR_DUPLICATE_DETECTION_ID and CONNECTION_ID_PROPERTY_NAME
- Refactored toAMQMessage by grouping methods for a better readability


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/franz1981/activemq-artemis open_wire_to_amqp

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1786.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1786


commit 4d493c0eae35fb0be2e3e212eadcb657346ff8f9
Author: Francesco Nigro 
Date:   2018-01-16T13:24:32Z

Avoided copy of CoreMessage when not needed and cached lambda on hot path

commit e4f9a4f9a0bc3277e580a3c014ba7d422597b81e
Author: Francesco Nigro 
Date:   2018-01-16T16:01:54Z

Refactored inbound in order to group help the JVM compile most used methods

commit 82d1cd8c5489c106b5ff9e22b264216988b7986e
Author: Francesco Nigro 
Date:   2018-01-16T17:40:06Z

Optimized SimpleString::split because heavily used into AddressImpl::new

commit df18521298e8866d1e75e299336d25845b97f663
Author: Francesco Nigro 
Date:   2018-01-16T20:27:22Z

Added existing queues cache to avoid multiple expensive checkCreateQueue 
calls

commit 74eb72bffa7c723e714d4744fcc9e7207b5fe92a
Author: Francesco Nigro 
Date:   2018-01-16T21:24:08Z

used SimpleString to avoid translations on CoreMessage

commit 5752e30a9363bc473bd52e3ae09c677687ad1ab2
Author: Francesco Nigro 
Date:   2018-01-16T23:37:24Z

cached Notification Destination check on AMQConsumer

commit 94c9de15fb729d9e8294166cc9d48fdd0e09
Author: Francesco Nigro 
Date:   2018-01-17T07:29:28Z

Used SimpleString explicitly with HDR_DUPLICATE_DETECTION_ID and 
CONNECTION_ID_PROPERTY_NAME

commit 1f53625848ae10269c9748b4125848609fd3854b
Author: Francesco Nigro 
Date:   2018-01-17T13:37:08Z

Refactored toAMQMessage by grouping methods for a better readability




> OpenWire improvements
> -
>
> Key: ARTEMIS-1616
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1616
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>Reporter: Francesco Nigro
>Assignee: Francesco Nigro
>Priority: Major
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)