Skip to content

Commit

Permalink
fix: do not create constant pool key eagerly
Browse files Browse the repository at this point in the history
Stops creating a constant pool key eagerly as it was being created 11
times for a simple click listener after each data expression was evaluated.

Next step is to also make it not create the MessageDigest over and over again.

Part of #7826
  • Loading branch information
pleku committed Oct 26, 2021
1 parent 4359497 commit 6b90b08
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
*/
public class ConstantPoolKey implements Serializable {
private final JsonValue json;
private final String id;
private String id;

/**
* Creates a new constant pool key for the given JSON value. The value
Expand All @@ -53,8 +53,6 @@ public class ConstantPoolKey implements Serializable {
public ConstantPoolKey(JsonValue json) {
assert json != null;
this.json = json;

id = calculateHash(json);
}

/**
Expand All @@ -63,11 +61,14 @@ public ConstantPoolKey(JsonValue json) {
* @return the id used to identify this value
*/
public String getId() {
if (id == null) {
id = calculateHash(json);
}
return id;
}

/**
* Exports the this key into a JSON object to send to the client. This
* Exports this key into a JSON object to send to the client. This
* method should be called only by the {@link ConstantPool} instance that
* manages this value. It may be called multiple times.
*
Expand All @@ -76,9 +77,7 @@ public String getId() {
* <code>null</code>
*/
public void export(JsonObject clientConstantPoolUpdate) {
assert id.equals(calculateHash(json)) : "Json value has been changed";

clientConstantPoolUpdate.put(id, json);
clientConstantPoolUpdate.put(getId(), json);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@

import org.junit.Assert;
import org.junit.Test;
import org.mockito.MockedStatic;
import org.mockito.Mockito;

import com.vaadin.flow.component.ComponentTest.TestComponent;
import com.vaadin.flow.dom.Element;
import com.vaadin.flow.internal.JsonCodec;
import com.vaadin.flow.internal.MessageDigestUtil;
import com.vaadin.flow.internal.nodefeature.ElementListenerMap;
import com.vaadin.flow.shared.Registration;

Expand Down Expand Up @@ -86,6 +89,11 @@ public boolean equals(Object obj) {
}
}

@Tag("button")
private class TestButton extends Component implements ClickNotifier {

}

private void fireDomEvent(Component component, String domEvent,
JsonObject eventData) {
Element e = component.getElement();
Expand Down Expand Up @@ -482,4 +490,16 @@ public void eventUnregisterListener_outsideListenerTwiceThrows() {
c.fireEvent(new ServerEvent(c, new BigDecimal(0)));
storedEvent.get().unregisterListener();
}

@Test // #7826
public void addListener_eventDataExpressionsPresent_constantPoolKeyNotCreatedAfterEachExpression() {
final TestButton button = new TestButton();
try (MockedStatic<MessageDigestUtil> util = Mockito.mockStatic(MessageDigestUtil.class)){
util.when(() -> MessageDigestUtil.sha256(Mockito.anyString())).thenReturn(new byte[]{1,1,1,1,1,1,1,1,1,1,1,});
button.addClickListener(event -> {
});
util.verifyNoInteractions();
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright 2000-2021 Vaadin Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

package com.vaadin.flow.uitest.ui;

import com.vaadin.flow.component.UI;
import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.component.html.H1;
import com.vaadin.flow.component.html.NativeButton;
import com.vaadin.flow.router.Route;

@Route("performance/constant-pool")
public class ConstantPoolPerformanceView extends AbstractDivView {

Div container = new Div();
Div notification = new Div();
public ConstantPoolPerformanceView() {
NativeButton btn = new NativeButton("refresh");
btn.addClickListener(e->{
container.removeAll();
for (int i = 0; i <1000; i++) {
container.add(new H1("Headline " + i));
container.add(new NativeButton("BTN " + i, e2 -> notification.setText("clicked")));
}
UI.getCurrent().getPage().executeJs("setTimeout(function(){$0.click()},2000)", btn);
});
add(btn,notification,container);
}

}
7 changes: 6 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,11 @@
<artifactId>mockito-core</artifactId>
<version>3.12.4</version>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-inline</artifactId>
<version>3.12.4</version>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-all</artifactId>
Expand All @@ -275,7 +280,7 @@
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<artifactId>mockito-inline</artifactId>
<scope>test</scope>
</dependency>
<dependency>
Expand Down

0 comments on commit 6b90b08

Please sign in to comment.