You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by GitBox <gi...@apache.org> on 2022/04/19 18:52:42 UTC
[GitHub] [wicket] theigl commented on a diff in pull request #513: More efficient hashcode computations
theigl commented on code in PR #513:
URL: https://github.com/apache/wicket/pull/513#discussion_r853384634
##########
wicket-core/src/main/java/org/apache/wicket/Component.java:
##########
@@ -2669,7 +2669,7 @@ public void internalRenderHead(final HtmlHeaderContainer container)
if (response.wasRendered(behavior) == false)
{
behavior.renderHead(this, response);
- List<IClusterable> pair = Arrays.asList(this, behavior);
Review Comment:
Please revert the commit from this PR.
##########
wicket-core/src/main/java/org/apache/wicket/markup/head/AbstractJavaScriptReferenceHeaderItem.java:
##########
@@ -135,6 +135,16 @@ public boolean equals(Object o)
@Override
public int hashCode()
{
- return Objects.hash(async, defer, charset);
+ // original code was: return Objects.hash(async, defer, charset);
+ // This returns the same hash code value, but doesn't allocate an Object[3],
+ // which consumes 32 bytes, and doesn't need to autobox the two booleans.
+ // This method is called very often, especially when a containing Map or Set is resized,
+ // and this version will run faster and save a lot of memory.
+ // It is possible that Escape Analysis would elide the creation of the Object[],
+ // but we do not see that in our running applications with the latest JDK.
+ return 31*31*31 +
+ 31*31*Boolean.hashCode(async) +
+ 31*Boolean.hashCode(defer) +
+ ((charset != null) ? charset.hashCode() : 0);
Review Comment:
I find this quite hard to read. Can you please use IntelliJ or Eclipse to generate the code. IntelliJ looks like this:
```
@Override
public int hashCode()
{
int result = super.hashCode();
result = 31 * result + (async ? 1 : 0);
result = 31 * result + (defer ? 1 : 0);
result = 31 * result + (charset != null ? charset.hashCode() : 0);
return result;
}
```
##########
wicket-core/src/main/java/org/apache/wicket/markup/head/AbstractJavaScriptReferenceHeaderItem.java:
##########
@@ -135,6 +135,16 @@ public boolean equals(Object o)
@Override
public int hashCode()
{
- return Objects.hash(async, defer, charset);
+ // original code was: return Objects.hash(async, defer, charset);
+ // This returns the same hash code value, but doesn't allocate an Object[3],
+ // which consumes 32 bytes, and doesn't need to autobox the two booleans.
+ // This method is called very often, especially when a containing Map or Set is resized,
+ // and this version will run faster and save a lot of memory.
+ // It is possible that Escape Analysis would elide the creation of the Object[],
+ // but we do not see that in our running applications with the latest JDK.
Review Comment:
This comment should go into the corresponding Jira ticket. A simple comment like this should be enough:
```
// Not using `Objects.hash` for performance reasons
```
##########
wicket-core/src/main/java/org/apache/wicket/markup/head/AbstractJavaScriptReferenceHeaderItem.java:
##########
@@ -135,6 +135,16 @@ public boolean equals(Object o)
@Override
public int hashCode()
{
- return Objects.hash(async, defer, charset);
+ // original code was: return Objects.hash(async, defer, charset);
+ // This returns the same hash code value, but doesn't allocate an Object[3],
+ // which consumes 32 bytes, and doesn't need to autobox the two booleans.
+ // This method is called very often, especially when a containing Map or Set is resized,
+ // and this version will run faster and save a lot of memory.
+ // It is possible that Escape Analysis would elide the creation of the Object[],
+ // but we do not see that in our running applications with the latest JDK.
+ return 31*31*31 +
+ 31*31*Boolean.hashCode(async) +
+ 31*Boolean.hashCode(defer) +
+ ((charset != null) ? charset.hashCode() : 0);
Review Comment:
I find this quite hard to read. Can you please use IntelliJ or Eclipse to generate the code. IntelliJ looks like this:
```java
@Override
public int hashCode()
{
int result = super.hashCode();
result = 31 * result + (async ? 1 : 0);
result = 31 * result + (defer ? 1 : 0);
result = 31 * result + (charset != null ? charset.hashCode() : 0);
return result;
}
```
##########
wicket-core/src/main/java/org/apache/wicket/markup/head/AbstractJavaScriptReferenceHeaderItem.java:
##########
@@ -135,6 +135,16 @@ public boolean equals(Object o)
@Override
public int hashCode()
{
- return Objects.hash(async, defer, charset);
+ // original code was: return Objects.hash(async, defer, charset);
+ // This returns the same hash code value, but doesn't allocate an Object[3],
+ // which consumes 32 bytes, and doesn't need to autobox the two booleans.
+ // This method is called very often, especially when a containing Map or Set is resized,
+ // and this version will run faster and save a lot of memory.
+ // It is possible that Escape Analysis would elide the creation of the Object[],
+ // but we do not see that in our running applications with the latest JDK.
Review Comment:
This comment should go into the corresponding Jira ticket. A simple comment like this should be enough:
```
// Not using `Objects.hash` for performance reasons
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org