Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce memory overhead caused by default collection sizes #4562

Merged
merged 1 commit into from
Aug 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 Expand Up @@ -151,7 +152,7 @@ private <T extends ComponentEvent<?>> 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);
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,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
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