From 1be700a3fda717b650d966b0f723c68c2ee2f928 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leif=20=C3=85strand?= Date: Tue, 28 Aug 2018 12:23:55 +0300 Subject: [PATCH] Reduce memory overhead caused by default collection sizes There are various collections that are usually not used at all or with only a small number of items. Allocating the default sizes for such collections wastes quite some memory: HashSet and HashMap allocate for 16 items by default, ArrayList for 10 items and IdentityHashMap for 32. Reduces BasicElementView memory use from 429706 to 419682 bytes and the memory use in BeverageBuddy with an edit dialog open from 262257 to 251569 bytes. --- .../com/vaadin/flow/data/binder/Binder.java | 45 +++++++++++++------ .../flow/component/ComponentEventBus.java | 5 ++- .../nodefeature/ElementListenerMap.java | 39 +++++++++++----- .../nodefeature/ElementPropertyMap.java | 18 +++++--- .../flow/internal/nodefeature/NodeList.java | 6 +-- 5 files changed, 78 insertions(+), 35 deletions(-) diff --git a/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java b/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java index 630e3d64b84..db92ebde360 100644 --- a/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java +++ b/flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java @@ -38,7 +38,6 @@ import java.util.stream.Stream; import com.googlecode.gentyref.GenericTypeReflector; - import com.vaadin.external.org.slf4j.Logger; import com.vaadin.external.org.slf4j.LoggerFactory; import com.vaadin.flow.component.Component; @@ -786,7 +785,9 @@ public Binding bind(ValueProvider getter, getBinder().fireStatusChangeEvent(false); bound = true; - getBinder().incompleteBindings.remove(getField()); + if (getBinder().incompleteBindings != null) { + getBinder().incompleteBindings.remove(getField()); + } return binding; } @@ -823,7 +824,10 @@ public Binding bind(String propertyName) { getBinder().boundProperties.put(propertyName, binding); return binding; } finally { - getBinder().incompleteMemberFieldBindings.remove(getField()); + if (getBinder().incompleteMemberFieldBindings != null) { + getBinder().incompleteMemberFieldBindings + .remove(getField()); + } } } @@ -1302,17 +1306,18 @@ void setIdentity() { */ private final Map> boundProperties = new HashMap<>(); - private final Map, BindingBuilder> incompleteMemberFieldBindings = new IdentityHashMap<>(); + private Map, BindingBuilder> incompleteMemberFieldBindings; private BEAN bean; private final Collection> bindings = new ArrayList<>(); - private final Map, BindingBuilder> incompleteBindings = new IdentityHashMap<>(); + private Map, BindingBuilder> incompleteBindings; private final List> validators = new ArrayList<>(); - private final Map, ConverterDelegate> initialConverters = new IdentityHashMap<>(); + private final Map, ConverterDelegate> initialConverters = new IdentityHashMap<>( + 4); private HashMap, List>> listeners = new HashMap<>(); @@ -1496,6 +1501,9 @@ public BindingBuilder forField( */ public BindingBuilder forMemberField( HasValue field) { + if (incompleteMemberFieldBindings == null) { + incompleteMemberFieldBindings = new IdentityHashMap<>(8); + } incompleteMemberFieldBindings.put(field, null); return forField(field); } @@ -2177,8 +2185,8 @@ public Registration addStatusChangeListener(StatusChangeListener listener) { */ protected Registration addListener(Class eventType, SerializableConsumer method) { - List> list = listeners.computeIfAbsent(eventType, - key -> new ArrayList<>()); + List> list = listeners + .computeIfAbsent(eventType, key -> new ArrayList<>()); list.add(method); return () -> list.remove(method); } @@ -2188,8 +2196,8 @@ protected Registration addListener(Class eventType, *

* Added listener is notified every time whenever any bound field value is * changed, i.e. the UI component value was changed, passed all the - * conversions and validations then propagated to the bound bean field. The same - * functionality can be achieved by adding a + * conversions and validations then propagated to the bound bean field. The + * same functionality can be achieved by adding a * {@link ValueChangeListener} to all fields in the {@link Binder}. *

* The listener is added to all fields regardless of whether the method is @@ -2229,9 +2237,13 @@ protected BindingBuilder createBinding( BindingValidationStatusHandler handler) { BindingBuilder newBinding = doCreateBinding(field, converter, handler); - if (incompleteMemberFieldBindings.containsKey(field)) { + if (incompleteMemberFieldBindings != null + && incompleteMemberFieldBindings.containsKey(field)) { incompleteMemberFieldBindings.put(field, newBinding); } + if (incompleteBindings == null) { + incompleteBindings = new IdentityHashMap<>(8); + } incompleteBindings.put(field, newBinding); return newBinding; } @@ -2468,14 +2480,15 @@ private Converter createNullRepresentationA * if this binder has incomplete bindings */ private void checkBindingsCompleted(String methodName) { - if (!incompleteMemberFieldBindings.isEmpty()) { + if (!(incompleteMemberFieldBindings == null + || incompleteMemberFieldBindings.isEmpty())) { throw new IllegalStateException( "All bindings created with forMemberField must " + "be completed with bindInstanceFields before calling " + methodName); } - if (!incompleteBindings.isEmpty()) { + if (!(incompleteBindings == null || incompleteBindings.isEmpty())) { throw new IllegalStateException( "All bindings created with forField must be completed before calling " + methodName); @@ -2540,7 +2553,8 @@ public void bindInstanceFields(Object objectWithMemberFields) { memberField, property, type))) .reduce(0, this::accumulate, Integer::sum); if (numberOfBoundFields == 0 && bindings.isEmpty() - && incompleteBindings.isEmpty()) { + && (incompleteBindings == null + || incompleteBindings.isEmpty())) { // Throwing here for incomplete bindings would be wrong as they // may be completed after this call. If they are not, setBean and // other methods will throw for those cases @@ -2568,6 +2582,9 @@ private int accumulate(int count, boolean value) { private BindingBuilder getIncompleteMemberFieldBinding( Field memberField, Object objectWithMemberFields) { + if (incompleteMemberFieldBindings == null) { + return null; + } return incompleteMemberFieldBindings .get(getMemberFieldValue(memberField, objectWithMemberFields)); } diff --git a/flow-server/src/main/java/com/vaadin/flow/component/ComponentEventBus.java b/flow-server/src/main/java/com/vaadin/flow/component/ComponentEventBus.java index 4b558523856..6cb9b75958a 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/ComponentEventBus.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/ComponentEventBus.java @@ -69,7 +69,8 @@ public ListenerWrapper(ComponentEventListener listener) { } // Package private to enable testing only - HashMap>, ArrayList>> componentEventData = new HashMap<>(); + HashMap>, ArrayList>> componentEventData = new HashMap<>( + 2); private Component component; @@ -151,7 +152,7 @@ private > Registration addListenerInternal( domListenerConsumer.accept(wrapper.domRegistration); } - componentEventData.computeIfAbsent(eventType, t -> new ArrayList<>()) + componentEventData.computeIfAbsent(eventType, t -> new ArrayList<>(1)) .add(wrapper); return () -> removeListener(eventType, wrapper); diff --git a/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementListenerMap.java b/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementListenerMap.java index 9d66b1cd7ab..91d1e57427f 100644 --- a/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementListenerMap.java +++ b/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementListenerMap.java @@ -129,9 +129,17 @@ public DomListenerRegistration addEventData(String eventData) { } if (eventDataExpressions == null) { - eventDataExpressions = new HashSet<>(); + eventDataExpressions = Collections.singleton(eventData); + } else { + if (eventDataExpressions.size() == 1) { + Set oldExpressions = eventDataExpressions; + // Don't use no-args or Collection constructors that + // allocate for 16 entries + eventDataExpressions = new HashSet<>(4); + eventDataExpressions.addAll(oldExpressions); + } + eventDataExpressions.add(eventData); } - eventDataExpressions.add(eventData); listenerMap.updateEventSettings(type); @@ -233,14 +241,22 @@ public DomListenerRegistration add(String eventType, assert eventType != null; assert listener != null; - if (listeners == null) { - listeners = new HashMap<>(); - } - if (!contains(eventType)) { - assert !listeners.containsKey(eventType); + assert listeners == null || !listeners.containsKey(eventType); + + ArrayList listenerList = new ArrayList<>( + 1); + + if (listeners == null) { + listeners = Collections.singletonMap(eventType, listenerList); + } else { + if (listeners.size() == 1 && !(listeners instanceof HashMap)) { + listeners = new HashMap<>(listeners); + } + + listeners.put(eventType, listenerList); + } - listeners.put(eventType, new ArrayList<>()); } DomEventListenerWrapper listenerWrapper = new DomEventListenerWrapper( @@ -337,10 +353,11 @@ private void removeListener(String eventType, // No more listeners of this type? if (listenerList.isEmpty()) { - listeners.remove(eventType); - - if (listeners.isEmpty()) { + if (listeners.size() == 1) { + assert listeners.containsKey(eventType); listeners = null; + } else { + listeners.remove(eventType); } // Remove from the set that is synchronized with the client diff --git a/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementPropertyMap.java b/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementPropertyMap.java index a19680f228a..6087a44fcc3 100644 --- a/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementPropertyMap.java +++ b/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/ElementPropertyMap.java @@ -85,8 +85,9 @@ public Runnable deferredUpdateFromClient(String key, Serializable value) { */ if (updateFromClientFilter != null && !updateFromClientFilter.test(key)) { - getLogger().warn("Ignoring model update for {}. " - + "For security reasons, the property must have a two-way binding in the template, be annotated with @{} in the model, or be defined as synchronized.", + getLogger().warn( + "Ignoring model update for {}. " + + "For security reasons, the property must have a two-way binding in the template, be annotated with @{} in the model, or be defined as synchronized.", key, AllowClientUpdates.class.getSimpleName()); return () -> { // nop @@ -134,11 +135,18 @@ public Registration addPropertyChangeListener(String name, PropertyChangeListener listener) { assert hasElement(); + List propertyListeners; if (listeners == null) { - listeners = new HashMap<>(); + propertyListeners = new ArrayList<>(1); + listeners = Collections.singletonMap(name, propertyListeners); + } else { + if (listeners.size() == 1 && !(listeners instanceof HashMap)) { + listeners = new HashMap<>(listeners); + } + propertyListeners = listeners.computeIfAbsent(name, + key -> new ArrayList<>(1)); } - List propertyListeners = listeners - .computeIfAbsent(name, key -> new ArrayList<>()); + propertyListeners.add(listener); return () -> propertyListeners.remove(listener); } diff --git a/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/NodeList.java b/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/NodeList.java index 5a4f0dc2097..fd841430ed2 100644 --- a/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/NodeList.java +++ b/flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/NodeList.java @@ -170,7 +170,7 @@ protected int size() { private void ensureValues() { if (values == null) { - values = new ArrayList<>(); + values = new ArrayList<>(1); } } @@ -329,7 +329,7 @@ public void collectChanges(Consumer collector) { if (isRemoveAllCalled && !hasRemoveAll) { changes = Stream - .concat(Stream.of(new ListClearChange(this)), + .concat(Stream.of(new ListClearChange<>(this)), allChanges.stream()) .filter(this::acceptChange).collect(Collectors.toList()); } else { @@ -398,7 +398,7 @@ protected void clear() { } isRemoveAllCalled = true; - addChange(new ListClearChange(this)); + addChange(new ListClearChange<>(this)); } /**