[GitHub] [groovy] blackdrag commented on a change in pull request #903: GROOVY-9060: Make the initial capacity of LinkedHashMap be power of 2…

2019-03-31 Thread GitBox
blackdrag commented on a change in pull request #903: GROOVY-9060: Make the 
initial capacity of LinkedHashMap be power of 2…
URL: https://github.com/apache/groovy/pull/903#discussion_r270663532
 
 

 ##
 File path: src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java
 ##
 @@ -396,10 +402,33 @@ public static List createList(Object[] values) {
 return answer;
 }
 
+/**
+ * Get the initial capacity of hash map, which is the closest power of 2 
to the entry count.
+ * (SEE 
https://stackoverflow.com/questions/8352378/why-does-hashmap-require-that-the-initial-capacity-be-a-power-of-two)
+ *
+ * e.g.
+ * 1: 1
+ * 2: 2
+ * 3: 4
+ * 4: 4
+ * 5: 8
+ * 6: 8
+ * 7: 8
+ * 8: 8
+ * 9: 16
+ * ...
+ *
+ * @param initialEntryCnt the initial entry count
+ * @return the initial capacity
+ */
+public static int initialCapacity(int initialEntryCnt) {
+return (int) pow(2, ceil(log(initialEntryCnt) / LN2));
 
 Review comment:
   ups, I did misread the documentation. 
Integer.highestOneBit​(initialEntryCnt)*2 then


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [groovy] blackdrag commented on a change in pull request #903: GROOVY-9060: Make the initial capacity of LinkedHashMap be power of 2…

2019-03-31 Thread GitBox
blackdrag commented on a change in pull request #903: GROOVY-9060: Make the 
initial capacity of LinkedHashMap be power of 2…
URL: https://github.com/apache/groovy/pull/903#discussion_r270663532
 
 

 ##
 File path: src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java
 ##
 @@ -396,10 +402,33 @@ public static List createList(Object[] values) {
 return answer;
 }
 
+/**
+ * Get the initial capacity of hash map, which is the closest power of 2 
to the entry count.
+ * (SEE 
https://stackoverflow.com/questions/8352378/why-does-hashmap-require-that-the-initial-capacity-be-a-power-of-two)
+ *
+ * e.g.
+ * 1: 1
+ * 2: 2
+ * 3: 4
+ * 4: 4
+ * 5: 8
+ * 6: 8
+ * 7: 8
+ * 8: 8
+ * 9: 16
+ * ...
+ *
+ * @param initialEntryCnt the initial entry count
+ * @return the initial capacity
+ */
+public static int initialCapacity(int initialEntryCnt) {
+return (int) pow(2, ceil(log(initialEntryCnt) / LN2));
 
 Review comment:
   ups, I did misread the documentation. 
Integer.highestOneBit​(initialEntryCnt)*2 then. It does deliver 8 for 4 
entries, instead of 4, but there is a possibility the map will rehash at full 
capacity. Actually it might rehash much earlier (some maps rehash on 70% 
fillrate for example). 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [groovy] danielsun1106 commented on a change in pull request #903: GROOVY-9060: Make the initial capacity of LinkedHashMap be power of 2…

2019-03-31 Thread GitBox
danielsun1106 commented on a change in pull request #903: GROOVY-9060: Make the 
initial capacity of LinkedHashMap be power of 2…
URL: https://github.com/apache/groovy/pull/903#discussion_r270664227
 
 

 ##
 File path: src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java
 ##
 @@ -396,10 +402,33 @@ public static List createList(Object[] values) {
 return answer;
 }
 
+/**
+ * Get the initial capacity of hash map, which is the closest power of 2 
to the entry count.
+ * (SEE 
https://stackoverflow.com/questions/8352378/why-does-hashmap-require-that-the-initial-capacity-be-a-power-of-two)
+ *
+ * e.g.
+ * 1: 1
+ * 2: 2
+ * 3: 4
+ * 4: 4
+ * 5: 8
+ * 6: 8
+ * 7: 8
+ * 8: 8
+ * 9: 16
+ * ...
+ *
+ * @param initialEntryCnt the initial entry count
+ * @return the initial capacity
+ */
+public static int initialCapacity(int initialEntryCnt) {
+return (int) pow(2, ceil(log(initialEntryCnt) / LN2));
 
 Review comment:
   I will tweak the current implementation according to suggestion, which will 
be more efficient 😄


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [groovy] danielsun1106 commented on a change in pull request #903: GROOVY-9060: Make the initial capacity of LinkedHashMap be power of 2…

2019-03-31 Thread GitBox
danielsun1106 commented on a change in pull request #903: GROOVY-9060: Make the 
initial capacity of LinkedHashMap be power of 2…
URL: https://github.com/apache/groovy/pull/903#discussion_r270664227
 
 

 ##
 File path: src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java
 ##
 @@ -396,10 +402,33 @@ public static List createList(Object[] values) {
 return answer;
 }
 
+/**
+ * Get the initial capacity of hash map, which is the closest power of 2 
to the entry count.
+ * (SEE 
https://stackoverflow.com/questions/8352378/why-does-hashmap-require-that-the-initial-capacity-be-a-power-of-two)
+ *
+ * e.g.
+ * 1: 1
+ * 2: 2
+ * 3: 4
+ * 4: 4
+ * 5: 8
+ * 6: 8
+ * 7: 8
+ * 8: 8
+ * 9: 16
+ * ...
+ *
+ * @param initialEntryCnt the initial entry count
+ * @return the initial capacity
+ */
+public static int initialCapacity(int initialEntryCnt) {
+return (int) pow(2, ceil(log(initialEntryCnt) / LN2));
 
 Review comment:
   I will tweak the current implementation according to your suggestion, which 
will be more efficient 😄


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Created] (GROOVY-9061) Failing tests containing @Grab statements within nested scripts fail in JDK12

2019-03-31 Thread Paul King (JIRA)
Paul King created GROOVY-9061:
-

 Summary: Failing tests containing @Grab statements within nested 
scripts fail in JDK12
 Key: GROOVY-9061
 URL: https://issues.apache.org/jira/browse/GROOVY-9061
 Project: Groovy
  Issue Type: Bug
Reporter: Paul King
Assignee: Paul King


Seems to be due to the class java.security.AccessController now appearing in 
the execution stack whereas it doesn't up until JDK11. This breaks the current 
logic for ReflectionUtils#getCallingClass that Grab uses to find the groovy 
class loader.



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


[jira] [Updated] (GROOVY-9061) Tests containing @Grab statements within nested scripts fail in JDK12

2019-03-31 Thread Paul King (JIRA)


 [ 
https://issues.apache.org/jira/browse/GROOVY-9061?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Paul King updated GROOVY-9061:
--
Summary: Tests containing @Grab statements within nested scripts fail in 
JDK12  (was: Failing tests containing @Grab statements within nested scripts 
fail in JDK12)

> Tests containing @Grab statements within nested scripts fail in JDK12
> -
>
> Key: GROOVY-9061
> URL: https://issues.apache.org/jira/browse/GROOVY-9061
> Project: Groovy
>  Issue Type: Bug
>Reporter: Paul King
>Assignee: Paul King
>Priority: Major
>
> Seems to be due to the class java.security.AccessController now appearing in 
> the execution stack whereas it doesn't up until JDK11. This breaks the 
> current logic for ReflectionUtils#getCallingClass that Grab uses to find the 
> groovy class loader.



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


[jira] [Resolved] (GROOVY-9061) Tests containing @Grab statements within nested scripts fail in JDK12

2019-03-31 Thread Paul King (JIRA)


 [ 
https://issues.apache.org/jira/browse/GROOVY-9061?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Paul King resolved GROOVY-9061.
---
   Resolution: Fixed
Fix Version/s: 2.5.7
   3.0.0-beta-1

> Tests containing @Grab statements within nested scripts fail in JDK12
> -
>
> Key: GROOVY-9061
> URL: https://issues.apache.org/jira/browse/GROOVY-9061
> Project: Groovy
>  Issue Type: Bug
>Reporter: Paul King
>Assignee: Paul King
>Priority: Major
> Fix For: 3.0.0-beta-1, 2.5.7
>
>
> Seems to be due to the class java.security.AccessController now appearing in 
> the execution stack whereas it doesn't up until JDK11. This breaks the 
> current logic for ReflectionUtils#getCallingClass that Grab uses to find the 
> groovy class loader.



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


[jira] [Updated] (GROOVY-9061) Tests containing @Grab statements within nested scripts fail in JDK12

2019-03-31 Thread Paul King (JIRA)


 [ 
https://issues.apache.org/jira/browse/GROOVY-9061?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Paul King updated GROOVY-9061:
--
Affects Version/s: 3.0.0-alpha-4
   2.5.6

> Tests containing @Grab statements within nested scripts fail in JDK12
> -
>
> Key: GROOVY-9061
> URL: https://issues.apache.org/jira/browse/GROOVY-9061
> Project: Groovy
>  Issue Type: Bug
>Affects Versions: 3.0.0-alpha-4, 2.5.6
>Reporter: Paul King
>Assignee: Paul King
>Priority: Major
> Fix For: 3.0.0-beta-1, 2.5.7
>
>
> Seems to be due to the class java.security.AccessController now appearing in 
> the execution stack whereas it doesn't up until JDK11. This breaks the 
> current logic for ReflectionUtils#getCallingClass that Grab uses to find the 
> groovy class loader.



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


[GitHub] [groovy] asfgit closed pull request #903: GROOVY-9060: Make the initial capacity of LinkedHashMap be power of 2…

2019-03-31 Thread GitBox
asfgit closed pull request #903: GROOVY-9060: Make the initial capacity of 
LinkedHashMap be power of 2…
URL: https://github.com/apache/groovy/pull/903
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Resolved] (GROOVY-9060) Make the initial capacity of LinkedHashMap be power of 2 for better performance

2019-03-31 Thread Daniel Sun (JIRA)


 [ 
https://issues.apache.org/jira/browse/GROOVY-9060?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Daniel Sun resolved GROOVY-9060.

Resolution: Fixed

Fixed by 
https://github.com/apache/groovy/commit/5c8f08633a320be8836a249d577a549fe5966b76

> Make the initial capacity of LinkedHashMap be power of 2 for better 
> performance
> ---
>
> Key: GROOVY-9060
> URL: https://issues.apache.org/jira/browse/GROOVY-9060
> Project: Groovy
>  Issue Type: Improvement
>Reporter: Daniel Sun
>Assignee: Daniel Sun
>Priority: Major
> Fix For: 3.0.0-beta-1
>
>  Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> {\{LinkedHashMap}} is a subclass of \{{HashMap}}, so its initial capacity 
> should be power of 2 too.
> SEE: 
> [https://stackoverflow.com/questions/8352378/why-does-hashmap-require-that-the-initial-capacity-be-a-power-of-two]
>  



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