Skip to content

Commit

Permalink
Reduce memory overhead caused by default collection sizes
Browse files Browse the repository at this point in the history
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 290239 to
244149 bytes.
  • Loading branch information
Legioth committed Aug 28, 2018
1 parent db73c7b commit 8425a19
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 34 deletions.
45 changes: 31 additions & 14 deletions flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -786,7 +785,9 @@ public Binding<BEAN, TARGET> bind(ValueProvider<BEAN, TARGET> getter,
getBinder().fireStatusChangeEvent(false);

bound = true;
getBinder().incompleteBindings.remove(getField());
if (getBinder().incompleteBindings != null) {
getBinder().incompleteBindings.remove(getField());
}

return binding;
}
Expand Down Expand Up @@ -823,7 +824,10 @@ public Binding<BEAN, TARGET> bind(String propertyName) {
getBinder().boundProperties.put(propertyName, binding);
return binding;
} finally {
getBinder().incompleteMemberFieldBindings.remove(getField());
if (getBinder().incompleteMemberFieldBindings != null) {
getBinder().incompleteMemberFieldBindings
.remove(getField());
}
}
}

Expand Down Expand Up @@ -1302,17 +1306,18 @@ void setIdentity() {
*/
private final Map<String, Binding<BEAN, ?>> boundProperties = new HashMap<>();

private final Map<HasValue<?, ?>, BindingBuilder<BEAN, ?>> incompleteMemberFieldBindings = new IdentityHashMap<>();
private Map<HasValue<?, ?>, BindingBuilder<BEAN, ?>> incompleteMemberFieldBindings;

private BEAN bean;

private final Collection<Binding<BEAN, ?>> bindings = new ArrayList<>();

private final Map<HasValue<?, ?>, BindingBuilder<BEAN, ?>> incompleteBindings = new IdentityHashMap<>();
private Map<HasValue<?, ?>, BindingBuilder<BEAN, ?>> incompleteBindings;

private final List<Validator<? super BEAN>> validators = new ArrayList<>();

private final Map<HasValue<?, ?>, ConverterDelegate<?>> initialConverters = new IdentityHashMap<>();
private final Map<HasValue<?, ?>, ConverterDelegate<?>> initialConverters = new IdentityHashMap<>(
4);

private HashMap<Class<?>, List<SerializableConsumer<?>>> listeners = new HashMap<>();

Expand Down Expand Up @@ -1496,6 +1501,9 @@ public <FIELDVALUE> BindingBuilder<BEAN, FIELDVALUE> forField(
*/
public <FIELDVALUE> BindingBuilder<BEAN, FIELDVALUE> forMemberField(
HasValue<?, FIELDVALUE> field) {
if (incompleteMemberFieldBindings == null) {
incompleteMemberFieldBindings = new IdentityHashMap<>(8);
}
incompleteMemberFieldBindings.put(field, null);
return forField(field);
}
Expand Down Expand Up @@ -2177,8 +2185,8 @@ public Registration addStatusChangeListener(StatusChangeListener listener) {
*/
protected <T> Registration addListener(Class<T> eventType,
SerializableConsumer<T> method) {
List<SerializableConsumer<?>> list = listeners.computeIfAbsent(eventType,
key -> new ArrayList<>());
List<SerializableConsumer<?>> list = listeners
.computeIfAbsent(eventType, key -> new ArrayList<>());
list.add(method);
return () -> list.remove(method);
}
Expand All @@ -2188,8 +2196,8 @@ protected <T> Registration addListener(Class<T> eventType,
* <p>
* 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}.
* <p>
* The listener is added to all fields regardless of whether the method is
Expand Down Expand Up @@ -2229,9 +2237,13 @@ protected <FIELDVALUE, TARGET> BindingBuilder<BEAN, TARGET> createBinding(
BindingValidationStatusHandler handler) {
BindingBuilder<BEAN, TARGET> 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;
}
Expand Down Expand Up @@ -2468,14 +2480,15 @@ private <FIELDVALUE> Converter<FIELDVALUE, FIELDVALUE> 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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2568,6 +2582,9 @@ private int accumulate(int count, boolean value) {

private BindingBuilder<BEAN, ?> getIncompleteMemberFieldBinding(
Field memberField, Object objectWithMemberFields) {
if (incompleteMemberFieldBindings == null) {
return null;
}
return incompleteMemberFieldBindings
.get(getMemberFieldValue(memberField, objectWithMemberFields));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ public ListenerWrapper(ComponentEventListener<T> listener) {
}

// Package private to enable testing only
HashMap<Class<? extends ComponentEvent<?>>, ArrayList<ListenerWrapper<?>>> componentEventData = new HashMap<>();
HashMap<Class<? extends ComponentEvent<?>>, ArrayList<ListenerWrapper<?>>> componentEventData = new HashMap<>(
2);

private Component component;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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);

Expand Down Expand Up @@ -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<DomEventListenerWrapper> 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(
Expand Down Expand Up @@ -337,10 +353,10 @@ private void removeListener(String eventType,

// No more listeners of this type?
if (listenerList.isEmpty()) {
listeners.remove(eventType);

if (listeners.isEmpty()) {
if (listeners.size() == 1 && listeners.containsKey(eventType)) {
listeners = null;
} else {
listeners.remove(eventType);
}

// Remove from the set that is synchronized with the client
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -134,11 +135,18 @@ public Registration addPropertyChangeListener(String name,
PropertyChangeListener listener) {
assert hasElement();

List<PropertyChangeListener> 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<PropertyChangeListener> propertyListeners = listeners
.computeIfAbsent(name, key -> new ArrayList<>());

propertyListeners.add(listener);
return () -> propertyListeners.remove(listener);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ protected int size() {

private void ensureValues() {
if (values == null) {
values = new ArrayList<>();
values = new ArrayList<>(1);
}
}

Expand Down Expand Up @@ -329,7 +329,7 @@ public void collectChanges(Consumer<NodeChange> collector) {

if (isRemoveAllCalled && !hasRemoveAll) {
changes = Stream
.concat(Stream.of(new ListClearChange<T>(this)),
.concat(Stream.of(new ListClearChange<>(this)),
allChanges.stream())
.filter(this::acceptChange).collect(Collectors.toList());
} else {
Expand Down Expand Up @@ -398,7 +398,7 @@ protected void clear() {
}

isRemoveAllCalled = true;
addChange(new ListClearChange<T>(this));
addChange(new ListClearChange<>(this));
}

/**
Expand Down

0 comments on commit 8425a19

Please sign in to comment.