Vaadin 7 Table.ColumnGenerator: How to destroy unnecessary generated object

Vaadin 6 / 7 / 8
1

Hi!

I use Table.ColumnGenerator interface to generate dynamic cell content for my Table object. The table is updated frequently. Heap dump shows that my table keeps all generated unnecessary cell objects. How to destroy such objects?

I have made sample maven project (http://files.rsdn.ru/12237/memoryleak.zip) which can be used to reproduce the problem:

unzip memoryleak.zip
cd memoryleak
mvn package
start mvn jetty:run
start http:\localhost:8080\memoryleak
"%JAVA_HOME%"\bin\jvisualvm

In jvisualvm you can see that checkbox amount contantly increases (GC cannot destroy such objects, because the objects belong to the table object)

The code which generates CheckBox object the following:

public Object generateCell(final AbstractSelect source, final Object itemId,
Object columnId) {
final CheckBox checkBox = new CheckBox();
Object value = source.getValue();
boolean checked = false;
if( value instanceof Set<?>) {
Set<?> set = (Set<?>)value;
checked = set.contains(itemId);
}
checkBox.setValue(checked);
checkBox.setImmediate(true);
checkBox.setData(itemId);
checkBox.addValueChangeListener(new Property.ValueChangeListener() {
private static final long serialVersionUID = 1L;

@Override
public void valueChange(
com.vaadin.data.Property.ValueChangeEvent event) {
if( checkBox.getData() == null )
return;
Object oldData = checkBox.getData();
// это предотвратит повторный вход
checkBox.setData(null);
Object value = event.getProperty().getValue();
if( value instanceof Boolean ) {
Boolean checked = (Boolean) value;
Object currentValue = source.getValue();
if( currentValue instanceof Set) {
Set set = (Set)currentValue;
Set newSet = new HashSet(set);
if( checked ) {
newSet.add(itemId);
} else
newSet.remove(itemId);
source.setValue(newSet);
} else {
source.setValue(itemId);
}
}
checkBox.setData(oldData);
}
});
ContextMenu contextMenu = new ContextMenu();
contextMenu.addItem("Выделить все").addItemClickListener(new SerializableContextMenuItemClickListener() {

private static final long serialVersionUID = 1L;

@Override
public void contextMenuItemClicked(ContextMenuItemClickEvent event) {
source.setValue(new HashSet(source.getItemIds()));
}
});
contextMenu.addItem("Снять выделение").addItemClickListener(new SerializableContextMenuItemClickListener() {

private static final long serialVersionUID = 1L;

@Override
public void contextMenuItemClicked(ContextMenuItemClickEvent event) {
source.setValue(null);
}
});
contextMenu.setAsContextMenuOf(checkBox);
source.addValueChangeListener(new Property.ValueChangeListener() {
private static final long serialVersionUID = 1L;

@Override
public void valueChange(
com.vaadin.data.Property.ValueChangeEvent event) {
Object value = event.getProperty().getValue();
Object oldData = checkBox.getData();
checkBox.setData(null);
if( value instanceof Set<?> ) {
Set<?> set = (Set<?>) value;
checkBox.setValue(set.contains(itemId));
} else {
if( value != null ) {
checkBox.setValue(value.equals(itemId));
}
}
checkBox.setData(oldData);
}
});
return checkBox;

Thanks in advance,
Anatoly.

2

Hi,

Every time a check box is generated you add a value change listener to the Table. The value change listener has a reference to the check box so both will be kept in memory forever, i.e. everytime a checkbox is generated you will have more listeners. These will never be removed so in addition to the mentioned memory leak you will have a very inefficient table if hundreds or thousands of listeners are triggered every time the value is changed.

You can probably add just one value change listener to the table and do whatever you need to do in that listener.

3

Artur, thank you so much! It seems to me I got right solution:


		table = new Table();
		table.setSizeFull();
		table.setSelectable(true);
		table.setMultiSelect(false);
		table.setImmediate(true);
		table.addValueChangeListener(new Property.ValueChangeListener() {
			
			@Override
			public void valueChange(ValueChangeEvent event) {
				table.markAsDirtyRecursive();
			}
		});
		table.addGeneratedColumn(SELECTOR, new Table.ColumnGenerator() {

			@Override
			public Object generateCell(final Table source, final Object itemId,
					Object columnId) {
				Object value = source.getValue();
				boolean checked = true;
				if( value instanceof Set ) {
					Set<?> set = (Set<?>) value;
					checked = set.contains(itemId);
				} else {
					checked = itemId.equals(value);
				}
				final CheckBox checkBox = new CheckBox(null, checked);
				checkBox.addValueChangeListener(new Property.ValueChangeListener() {
					private static final long serialVersionUID = 1L;

					@Override
					public void valueChange(
							com.vaadin.data.Property.ValueChangeEvent event) {
						Object value = source.getValue();
						Object newValue = null;
						if( value instanceof Set ) {
							Set<?> set = (Set<?>) value;
							Set<Object> newSet = new HashSet<Object>(set);
							if( newSet.contains(itemId) ) {
								newSet.remove(itemId);
							} else {
								newSet.add(itemId);
							}
							newValue = newSet;
						} else {
							if( !itemId.equals(value) ) {
								newValue = itemId;
							} 
						}
						source.setValue(newValue);
					}
				});
				return checkBox;
			}
		});