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

UIData performance and state saving improvements, results of some profiling sessions #2969

Closed
ren-zhijun-oracle opened this issue Jul 25, 2013 · 22 comments

Comments

@ren-zhijun-oracle
Copy link
Contributor

reduce table state

SavedState instances saved with default state

UIData.saveDescentantState() creates and keeps SavedState instances for each EditableValueHolder/UIForm per row, even if the final SavedState instance has the same values as the default initial values for the saved attributes. For a small table with 10 rows and 5 columns with h:inputText this results in about 10KB state.

Change: SavedState instances will only be kept on state if the EditableValueHolder/UIForm state differs from the init values of SavedState. With that change if all the h:inputText are valid no single SavedState instance ends up in state. Only if input validation fails the SavedState instances hold delta state and must be stored in state.
restoreDescendentState() must handle non-saved SavedState by calling resetValue() on EditableValueHolder. On UIForm setSubmitted(false) is enough since that attribute is transient.

UIInput/UIOutput resetValue() does not clear state

See JAVASERVERFACES_SPEC_PUBLIC-1211, standard saveState also processes all column component for rowIndex -1. If UIData.restoreDescendentState() calls UIInput.resetValue() and this would remove delta state or the 3 attributes value, valid and localValueSet instead of calling the set methods to set deltaState to the same values as the default values would be.

UIData performance improvement

When rowIndex is set, UIData saveDescendentState()/restoreDescendentState() walk the whole children and facets sub-trees of all rendered UIColumns (including header/footer facets of UIColumns) to find all EditableValueHolder and UIForm to save SavedState instances for the current row and restore the values for the new rowIndex.

Change: walk the UIColumn sub-tree's only once to collect the components to process for SavedState handling and those to simply reset the id on restore. Not all component id's must be reset, only the ones of ActionSources, NamingContainers and components with explicitly set id's. Processing these preparedLists is 3-4 times faster then walking the sub-trees for save/restore.

The attached changebundle.txt contains the collected changes.
Also attached is a PDF with memory profiling details (inputText-in-table-saves-default-state.pdf).
Additional attached is the small test application that was used for profiling (saved-state.zip)
A fourth attachment will follow with performance comparision details.

Best regards
Hanspeter

Affected Versions

[2.2.1]

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
Reported by dueni

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
Issue-Links:
blocks
JAVASERVERFACES_SPEC_PUBLIC-1211

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
dueni said:
Added new changebundle with latest improvements.
Also attached a word document (PDF) with details about the done state-saving profiling tests and conclusions.

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
File: inputText-in-table-saves-default-state.pdf
Attached By: dueni

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
dueni said:
If JAVASERVERFACES_SPEC_PUBLIC-1211 gets fixed UIData.restoreDescendantState() could use UIInput.resetValue() instead accessing UIInput's StateHelper to remove the states for the properties to reset.

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
dueni said:
attach saved-state.zip containing the test application

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
File: saved-state.zip
Attached By: dueni

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
File: changebundle.txt
Attached By: dueni

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
File: UIData-performance-improvement.pdf
Attached By: dueni

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
File: netbeans-profiling-UIData-JSF-2.2.1-versus-JSF-2.2.2-809.xlsx
Attached By: dueni

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
@manfredriem said:
Did you attach changebundle-2965-2.txt?

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
dueni said:
changebundle-2965-2 was added before I did some more rework and then also renamed the issue. With the rename I removed the previous attachments and added one single changebundle.txt containing all changes.

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
@manfredriem said:
Applied to 2.2 branch,

svn commit -m "Fixes https://java.net/jira/browse/JAVASERVERFACES-2965, r=mriem, reduce table state"
Sending jsf-api\src\main\java\javax\faces\component\UIData.java
Sending jsf-api\src\main\java\javax\faces\component\UIInput.java
Sending jsf-api\src\main\java\javax\faces\component\UIOutput.java
Transmitting file data ...
Committed revision 12426.

Thanks Hanspeter!

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
dougd said:
I have attached some code that demonstrates a failure calling invokeOnComponent for UIData.

public void uiDataInvokeOnComponentCheck(HttpServletRequest request,
			HttpServletResponse response) throws ServletException, IOException {
		final PrintWriter out = response.getWriter();
		out.println("beginning uiDataPositiveInvokeOnComponentTest");
		initDataModel();

		// Create UIData component 		UIData data = (UIData) createComponent();
		data.setFirst(3);
		data.setRows(5);
		data.setVar("foo");
		data.setValue(testDM);
		if (data.getValue() == null || testDM != data.getValue()) {
			out.println(JSFTestUtil.FAIL + JSFTestUtil.NL
					+ " Bad value for data.getValue()");
			return;
		}

		// build component tree 		UIViewRoot viewRoot = new UIViewRoot();
		viewRoot.setRenderKitId(RenderKitFactory.HTML_BASIC_RENDER_KIT);
		viewRoot.setViewId("/view");
		getFacesContext().setViewRoot(viewRoot);

		UIForm form1 = new UIForm();
		form1.setId("form1");
		viewRoot.getChildren().add(form1);
		setupTree(form1, data);

		// find the data component 		UIData data1 = (UIData) viewRoot.findComponent("form1:data");

		boolean found = false;
		data1.setRowIndex(3);
		if (data1.getRowIndex() != 3) {
			out.println(JSFTestUtil.FAIL + JSFTestUtil.NL
					+ "Unexpected value for getRowIndex() after setting row "
					+ "index." + JSFTestUtil.NL + "Expected value: 3"
					+ JSFTestUtil.NL + "Computed value: " + data1.getRowIndex());
			return;
		}

		found = viewRoot.invokeOnComponent(getFacesContext(),   //<--- This returns False if 																														//     setRowIndex does not																															//     match clientId given. 				"form1:data:4:commandHeader", new ContextCallback() {

					public void invokeContextCallback(FacesContext context,
							UIComponent component) {
						out.println("entering invokeContextCallback()");

						UIData data = (UIData) getNamingContainer(component);
						UIForm form = (UIForm) getNamingContainer(data);

						if (data.getRowIndex() != 4) {
							out.println(JSFTestUtil.FAIL + JSFTestUtil.NL
									+ " Incorrect row index" + JSFTestUtil.NL
									+ "Expected: 4" + JSFTestUtil.NL
									+ "Computed: " + data.getRowIndex());
						}

						if (!"form1".equals(form.getId())) {
							out.println(JSFTestUtil.FAIL + JSFTestUtil.NL
									+ " Incorrect form id" + JSFTestUtil.NL
									+ "Expected: form1" + JSFTestUtil.NL
									+ "Computed: " + form.getId());
						}

						if (!"commandHeader".equals(component.getId())) {
							out.println(JSFTestUtil.FAIL + JSFTestUtil.NL
									+ "Incorrect component id" + JSFTestUtil.NL
									+ "Expected: commandHeader"
									+ JSFTestUtil.NL + "Computed: "
									+ component.getId());
						}
					}
				});

		if (data1.getRowIndex() != 3) {
			out.println(JSFTestUtil.FAIL + JSFTestUtil.NL 
					+"Unexpected value for getRowIndex() after" 
					+ JSFTestUtil.NL + "invokeOnComponent" + JSFTestUtil.NL
					+ "Expected value: 3" + JSFTestUtil.NL
					+ "Computed value: " + data1.getRowIndex());
			return;
		}

		if (!found) {
			out.println(JSFTestUtil.FAIL + JSFTestUtil.NL
					+ "Call to invokeOnComponent() returned false");
			return;
		}

		out.println(JSFTestUtil.PASS);
	}
private void setupTree(UIComponent root, UIData data) {

		// Attach our UIData to the view root 		// UIData data = new UIData(); 		data.setId("data");
		root.getChildren().add(data);

		// Set up columns with facets and fields for each property 		UIColumn column;
		UICommand command;
		UIInput input;
		UIOutput output;
		UIOutput label;
		UIOutput constant;

		column = new UIColumn();
		column.setId("commandColumn");
		label = new UIOutput();
		label.setId("commandHeader");
		label.setValue("Command Header");
		column.getFacets().put("header", label);

		label = new UIOutput();
		label.setId("commandFooter");
		label.setValue("Command Footer");
		column.getFacets().put("footer", label);
		command = new UICommand();
		command.setId("command");
		command.setValueBinding("value",
				getApplication().createValueBinding("#{foo.command}"));
		column.getChildren().add(command);
		data.getChildren().add(column);
		command.addActionListener(new TCKDataActionListener());

		column = new UIColumn();
		column.setId("inputColumn");
		label = new UIOutput();
		label.setId("inputHeader");
		label.setValue("Input Header");
		column.getFacets().put("header", label);

		label = new UIOutput();
		label.setId("inputFooter");
		label.setValue("Input Footer");
		column.getFacets().put("footer", label);
		input = new UIInput();
		input.setId("input");
		input.setValueBinding("value",
				getApplication().createValueBinding("#{foo.input}"));
		column.getChildren().add(input);
		data.getChildren().add(column);

		input.addValidator(new TCKValidator());
		input.addValueChangeListener(new TCKValueChangeListener());

		column = new UIColumn();
		column.setId("outputColumn");
		label = new UIOutput();
		label.setId("outputHeader");
		label.setValue("Output Header");
		column.getFacets().put("header", label);

		label = new UIOutput();
		label.setId("outputFooter");
		label.setValue("Output Footer");
		column.getFacets().put("footer", label);

		output = new UIOutput();
		output.setId("output");
		output.setValueBinding("value",
				getApplication().createValueBinding("#{foo.output}"));
		column.getChildren().add(output);
		data.getChildren().add(column);

		column = new UIColumn();
		column.setId("constantColumn");
		label = new UIOutput();
		label.setId("constantHeader");
		label.setValue("Constant Header");
		column.getFacets().put("header", label);

		label = new UIOutput();
		label.setId("constantFooter");
		label.setValue("Constant Footer");
		column.getFacets().put("footer", label);
		constant = new UIOutput();
		constant.setId("constant");
		constant.setValue("Constant Value");
		column.getChildren().add(constant);
		data.getChildren().add(column);

	}

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
@manfredriem said:
Hanspeter,

It looks like the above example seems to fail in finding the component. I am not entirely sure
where exactly it is happening. It could be that invokeOnComponent is finding the component in
the skeleton tree and not in the row-indexed tree. When you have some time, can you please have
a look at it?

Thanks!

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
dueni said:
Hi Manfred,

I see where the problem lies. Unfortunately I do not have time today to work on this, so unfortunately there is no other chance then to revert this change for the current release and I need to do some rework.
I will work on this and it would be nice if we can make another try with next iteration.

Regards
Hanspeter

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
dueni said:
Added changebundle-revert-perf-changes.txt to revert the problematic performance optimizations and keep only the state reduction changes.

Maybe we should separate the performance aspects into a separate issue so we can deliver the state size reduction already.

regards
Hanspeter

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
File: changebundle-revert-perf-changes.txt
Attached By: dueni

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
@manfredriem said:
Applied to 2.2 branch,

svn commit -m "Fixes https://java.net/jira/browse/JAVASERVERFACES-2965, r=mriem, reverting performance changes, but keep state saving changes."
Sending jsf-api\src\main\java\javax\faces\component\UIData.java
Transmitting file data .
Committed revision 12469.

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
Marked as fixed on Tuesday, September 3rd 2013, 9:46:54 am

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
This issue was imported from java.net JIRA JAVASERVERFACES-2965

@ren-zhijun-oracle
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant