Generating swing components on fly slow

[Yahya Elyasse]

Hello,
I have a Java swing application that communicates with a server through HTTP client/server requests.
the server sends me an html stream. i parse this html response and convert it to swing components. it is mapping html input fields to swing components.
The solution is working but the performance is not satisfactory:
-parsing the html form took 47 secs
-generating and rendering swing components took 134 secs
so as you can see this is poor performance.
I'm wondering what is a way to speed up this process? is it possible to find a clever way to do this performance tunning: parsing html form and converting to swing components?

thanks.

[pete stein]

1) Are you using a SwingWorker object so that the stream reading and HTML parsing are done in a background thread, and the Swing rendering done on the EDT?
2) How do you know that it's the Swing rendering that takes 134 seconds? Have you run this through a profiler?

[Yahya Elyasse]

pete stein wrote:1) Are you using a SwingWorker object so that the stream reading and HTML parsing are done in a background thread, and the Swing rendering done on the EDT?

hmm no i think i don't do the parsing in swingworker . because i need first to parse and collect html input fields ..put them in an array then walk away this array and create corresponding swing components and add them to JPanel. if i run the html parser in a thread and the swing render in other thread i'm not sure to have all html input fields to map them to swing to generate. do you have an Idea how i can achieve this with a SwingWoker? combining html form parsing and swing rendering in two threads cooperating toghether?

pete stein wrote:
2) How do you know that it's the Swing rendering that takes 134 seconds? Have you run this through a profiler?

yes , i simply used System.currentTimeMills() before and after the method for parsing/rendring swing

[Maneesh Godbole]

Yahya Elyasse wrote: i parse this html response and convert it to swing components. it is mapping html input fields to swing components.

Is the HTML pure related content? Or does it also contain some more stuff not related to the UI? This could be one bottleneck.
When are you creating the Swing components? After parsing the HTML? Is it possible for you to create them first and then use from the cache?

Disclaimer: I have never tried this out myself. Please take it with a pinch of salt. I am just thinking out loud here.
Also google for XML-Swing frameworks. I read about one such framework recently, but cannot recall the name right now. Since its an existing framework, chances are they have already taken care of the optimization.

[Yahya Elyasse]

Maneesh Godbole wrote:

When are you creating the Swing components? After parsing the HTML? Is it possible for you to create them first and then use from the cache?

Yes after i finish parsing html i have an array which contains html input form fields i then go throught this array and generate corresponding swing components.

Maneesh Godbole wrote:
Disclaimer: I have never tried this out myself. Please take it with a pinch of salt. I am just thinking out loud here.
Also google for XML-Swing frameworks. I read about one such framework recently, but cannot recall the name right now. Since its an existing framework, chances are they have already taken care of the optimization.

Yes i'm aware of the xml swing converters like http://www.swixml.org/ . but the problem is that my java application is bounded to a HTTP server data which i have no control over and have to deal with HTML form sometimes and xml content in other times.
There is also the cobra and Lobo java libraries .but those will require me to change a lot of my code.and i don't think corba or lobo will increase much the performance.

[Rob Spoor]

No offence, but I think there must be a flaw in your program. 47 seconds to parse an HTML file? How large is that file, a few hundred megabytes? And over 2 minutes for generating and rendering components? My most heavy GUI ever created takes only a few seconds (5-10 max) to create the GUI, and that is one heavy GUI.

Can you show us your rendering and genering code?

[Yahya Elyasse]

Rob Prime wrote:No offence, but I think there must be a flaw in your program. 47 seconds to parse an HTML file? How large is that file, a few hundred megabytes? And over 2 minutes for generating and rendering components? My most heavy GUI ever created takes only a few seconds (5-10 max) to create the GUI, and that is one heavy GUI.

Can you show us your rendering and genering code?

hmm ,oops i think i didn't calculated the time right .actually i simply did something like :

   long ts=System.currentTimeMillis();
	     htmlForm.buildForm();
	     long te=System.currentTimeMillis();
	  _logger.info("Form building took "+(te-ts)/100+" secs"); 

but i see i did a mistake and had to divide by 1000 not 100.
so sorry for the mistake .the right timings are :
Form Parsing took 4 secs
Form building took 13 secs

so it takes about 17 secs to render the swing form. is this a reasonable timing? my client is complaining that the form rendering is a bit slow (here 17 secs)

[Ulf Dittmer]

4 seconds still sounds like a long time to parse a file. Which API are you using to do this? Does it include the time to download it? How big is the file? Is the code building some intermediate data structures in that time?

13 seconds, too, is on the high side for building a GUI. How many elements does the GUI have? Is there additional processing being done in that time?

[Yahya Elyasse]

ok here you are the two essential classes : class Forms.java for parsing html and class HTMLForm.java for creating swing GUI.
Hope you can give me Ideas how to improve rendering speed of the swing form converted from html form.
thanks


Forms.java
-------------

  import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.jdesktop.dom.SimpleDocument;
import org.jdesktop.dom.SimpleDocumentBuilder;
import org.jdesktop.http.Method;
import org.jdesktop.http.Response;
import org.jdesktop.swingx.html.form.Option;
import org.w3c.dom.Node;
import org.w3c.tidy.Tidy;

/**
 * Utility class. The primary benefit of this class is for automatically
 * creating a Form instance based on a given Response.
 * 
 * 
 */
public final class Forms {
	/**
	 * Creates and returns a {@link Form} implementation by parsing an HTML form
	 * from the given {@link Response}. The HTML form to use is determined by
	 * the <code>formId</code> param. If null, then the first form in the
	 * document will be used. If no document is available, an
	 * IllegalStateException will be thrown.
	 * 
	 * <p>
	 * Because this method will be working with random html, it is necessary to
	 * first run the response through JTidy...
	 */
	public static Form getForm(Response response, String expression) {
		try {
			String html = response.getBody();
			html = tidy(html);
			SimpleDocument dom = SimpleDocumentBuilder.simpleParse(html);
			return getForm(dom, response.getBaseUrl(), expression);
		} catch (Exception e) {
			throw new IllegalStateException(e);
		}
	}

	public static Form getForm(SimpleDocument dom, final String baseUrl,
			String expression) {
		try {
			// find the <form> element with the specified id
			Node n = dom.getElement(expression);
			if (n != null) {
				dom.getString("@name", n);
				final String action = dom.getString("@action", n);
				final String method = dom.getString("@method", n);

				Set<Input> inputs = new LinkedHashSet<Input>();
				Map<String, Set><String>> radioInputs = new HashMap<String, Set><String>>();
				Map<String, String> labels = new LinkedHashMap<String, String>();
				// now dig out all of the input fields
				for (Node inputNode : dom.getElements(expression

				+ "//textarea | " + expression + "//input | " + expression
						+ "//select | " + expression + "//label")) {
					/*
					 * If the input type is "textfield" or "checkbox" or
					 * "password" no need to do anything If the input type is
					 * "radiobutton" collesce it with all other "radiobutton"
					 * types with the same name If the node name is "select"
					 * Save the "name" attribute, and "id" if there is one Dig
					 * out the values for each of the <option> sub tags
					 */

					if ("label".equalsIgnoreCase(inputNode.getNodeName())) {
						final String id = dom.getString("@for", inputNode);
						String labelTxt = inputNode.getTextContent();
						labels.put(id, labelTxt);

					}
					if ("input".equalsIgnoreCase(inputNode.getNodeName())) {
						final String inputID = dom.getString("@id", inputNode);
						final String inputName = dom.getString("@name",
								inputNode);
						final String inputValue = dom.getString("@value",
								inputNode);
						final String inputType = dom.getString("@type",
								inputNode);
						final String checked = dom.getString("@checked",
								inputNode);
						boolean isChecked = (checked != null && checked
								.equals("checked")) ? true : false;

						if ("radio".equals(inputType)
								|| "checkbox".equals(inputType)) {
							Set<String> values = radioInputs.get(inputName);
							if (values == null) {
								values = new LinkedHashSet<String>();
								radioInputs.put(inputName, values);

							}
							values.add(inputValue);
							RadioInput r = new RadioInput(inputType, inputID,
									inputName, values);
							r.setValue(inputValue);
							r.setChecked(isChecked);
							inputs.add(r);
						} else if ("text".equals(inputType)
								|| "password".equals(inputType)) {
							final String inputSize = dom.getString("@size",
									inputNode);

							Text t = new Text(inputType, inputID, inputName,
									inputSize);
							t.setValue(inputValue);
							inputs.add(t);

						} else {
							inputs.add(new Input() {
								private String value = inputValue;

								public String getId() {
									return inputID;
								}

								public String getName() {
									return inputName;
								}

								public String getType() {
									return inputType;
								}

								public String getValue() {
									return value;
								}

								public void setValue(String value) {
									this.value = value;
								}
							});
						}

					} else if ("textarea".equalsIgnoreCase(inputNode
							.getNodeName())) {
						final String inputID = dom.getString("@id", inputNode);
						final String inputName = dom.getString("@name",
								inputNode);
						final String inputValue = dom.getString("@value",
								inputNode);
						final String rows = dom.getString("@rows", inputNode);
						final String cols = dom.getString("@cols", inputNode);
						TextArea t = new TextArea(inputID, inputName,
								inputValue, rows, cols);
						inputs.add(t);
					} else if ("select".equalsIgnoreCase(inputNode
							.getNodeName())) {
						List<Option> selectedVals = new ArrayList<Option>();
						final String inputID = dom.getString("@id", inputNode);
						dom.getString("@type", inputNode);
						final String selectName = dom.getString("@name",
								inputNode);
						final String multiple = dom.getString("@multiple",
								inputNode);
						Option selected = null;
						Set<Option> options = new LinkedHashSet<Option>();

						for (Node optionNode : dom.getElements("option",
								inputNode)) {
							String optionValue = dom.getString("@value",
									optionNode);
							String optionDisplay = optionNode.getTextContent();
							Option option = new Option(optionValue,
									optionDisplay);
							String optionSelected = dom.getString("@selected",
									optionNode);
							if ("selected".equals(optionSelected)) {
								selected = option;

								selectedVals.add(option);

							}
							options.add(option);
						}
						Select select = new Select(inputID, selectName, options);
						if (selected != null && multiple == null) {
							select.setValue(selected);
							inputs.add(select);
						}

						if (multiple != null) {
							XSelect selectMultiple = new XSelect(inputID,
									selectName, options);
							selectMultiple.setSelectedValues(selectedVals);
							inputs.add(selectMultiple);
						}

					}
				}

				/*
				 * //now add all the form inputs for the radios for (String
				 * radName : radioInputs.keySet()) { inputs.add(new
				 * AbstractRadioInput(radName, radioInputs.get(radName))); }
				 */
				inputs.add(new LabelInput(labels));
				return new AbstractForm(baseUrl, inputs) {
					public String getAction() {
						return action;
					}

					public Method getMethod() {
						return Method.valueOf(method.toUpperCase());
					}
				};
			} else {
				return null;
			}
		} catch (Exception e) {
			throw new IllegalStateException(e);
		}
	}

	public static Form getFormById(Response response, final String formId) {
		try {
			String html = response.getBody();
			html = tidy(html);
			SimpleDocument dom = SimpleDocumentBuilder.simpleParse(html);
			return getFormById(dom, response.getBaseUrl(), formId);
		} catch (Exception e) {
			throw new IllegalStateException(e);
		}
	}

	public static Form getFormById(SimpleDocument dom, String baseUrl,
			String formId) {
		return getForm(dom, baseUrl, "//form[@id='" + formId + "']");
	}

	public static Form getFormByIndex(Response response, int index) {
		try {
			String html = response.getBody();
			html = tidy(html);
			SimpleDocument dom = SimpleDocumentBuilder.simpleParse(html);
			return getFormByIndex(dom, response.getBaseUrl(), index);
		} catch (Exception e) {
			throw new IllegalStateException(e);
		}
	}

	public static Form getFormByIndex(SimpleDocument dom, String baseUrl,
			int index) {
		return getForm(dom, baseUrl, "//form[" + index + "]");
	}

	public static Form getFormByName(Response response, String name) {
		try {
			String html = response.getBody();
			html = tidy(html);
			SimpleDocument dom = SimpleDocumentBuilder.simpleParse(html);
			return getFormByName(dom, response.getBaseUrl(), name);
		} catch (Exception e) {
			throw new IllegalStateException(e);
		}
	}

	public static Form getFormByName(SimpleDocument dom, String baseUrl,
			String name) {
		return getForm(dom, baseUrl, "//form[@name='" + name + "']");
	}

	/**
	 * Runs JTidy on the source string, to produce the dest string.
	 */
	public static String tidy(String source) {
		try {
			Tidy tidy = new Tidy();
			tidy.setXHTML(true);
			// tidy.setSmartIndent(true);

			ByteArrayInputStream in = new ByteArrayInputStream(source
					.getBytes());
			ByteArrayOutputStream out = new ByteArrayOutputStream();
			tidy.parse(in, out);
			in.close();
			return out.toString();
		} catch (Exception e) {
			e.printStackTrace();
			return source;
		}
	}

	private Forms() {
	}
}  

HTMLForm.java
------------------

 import java.awt.Dimension;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.swing.ButtonGroup;
import javax.swing.DefaultListModel;
import javax.swing.JButton;
import javax.swing.JCheckBox;
import javax.swing.JComboBox;
import javax.swing.JComponent;
import javax.swing.JLabel;
import javax.swing.JList;
import javax.swing.JPasswordField;
import javax.swing.JRadioButton;
import javax.swing.JScrollPane;
import javax.swing.JTextArea;
import javax.swing.JTextField;
import javax.swing.ListSelectionModel;
import javax.swing.ScrollPaneConstants;

import net.java.dev.designgridlayout.DesignGridLayout;

import org.apache.log4j.Logger;
import org.jdesktop.dom.SimpleDocument;
import org.jdesktop.dom.SimpleDocumentBuilder;
import org.jdesktop.html.form.Form;
import org.jdesktop.html.form.Forms;
import org.jdesktop.html.form.Input;
import org.jdesktop.html.form.LabelInput;
import org.jdesktop.html.form.Paragraph;
import org.jdesktop.html.form.RadioInput;
import org.jdesktop.html.form.Select;
import org.jdesktop.html.form.Text;
import org.jdesktop.html.form.TextArea;
import org.jdesktop.html.form.Title;
import org.jdesktop.html.form.XSelect;
import org.jdesktop.http.Parameter;
import org.jdesktop.http.Response;
import org.jdesktop.swingx.html.form.JXHtmlForm;
import org.jdesktop.swingx.html.form.Option;
import org.w3c.dom.Node;

import com.wootic.uploader.api.UploaderAPI;
import com.wootic.uploader.exception.UploaderException;

/**
 * 
 * 
 */
public class HTMLForm extends SelfResizingPanel {
	/**
	 * 
	 */
	private static final long serialVersionUID = 1L;
	private static Logger _logger = Logger.getLogger(HTMLForm.class);

	/**
	 * @param args
	 */
	public static void main(String[] args) {
		// TODO Auto-generated method stub

	}

	private JXHtmlForm jxform;
	private DesignGridLayout layout;
	private ButtonGroup radioBG = new ButtonGroup();
	private Map<Input, JComponent> components = new LinkedHashMap<Input, JComponent>();
	private final static int defaultInputSize = 30;
	private int inputSize = defaultInputSize;
	private int wInsets;
	private int hInsets;
	private UploaderAPI api;
	private Form form;
	private JButton sendButton;

	private JLabel formMessage;

	/**
	 * 
	 */
	public HTMLForm(UploaderAPI api, Form form) {
		this.api = api;
		this.form = form;

	}

	private void addComponent(Input input) {
		String id = input.getId() == null ? "null" : input.getId();
		JComponent comp = components.get(input);
		if (comp == null)
			return;
		if (input.getType().equals("submit"))
			layout.row().center().add(comp);
		else
			layout.row().grid(this.createLabel(getLabelInput(), comp, id)).add(
					comp);
	}

	private int adjustInputSize(int size) {
		int res = size < defaultInputSize ? defaultInputSize : size;
		return res;

	}

	public void buildForm() throws UploaderException {
		add(getJXForm());
		String title = this.getTitle().getValue();
		if (title != null)
			MainWindow.getInstance().setTitle(title);

	}

	private JButton createButton(Input input) {

		logInputInfo(input);
		JButton b = new JButton(input.getValue());
		b.setPreferredSize(new Dimension(100, 30));

		b.putClientProperty(JXHtmlForm.NAME, input.getName());
		b.putClientProperty(JXHtmlForm.VALUE, input.getValue());
		/**/

		return b;

	}

	private JCheckBox createCheckBox(Input input) {
		logInputInfo(input);
		RadioInput c = (RadioInput) input;
		_logger.info("checkbox input value: " + c.getValue());

		JCheckBox checkbox = new JCheckBox(c.getValue(), c.isChecked());

		checkbox.putClientProperty(JXHtmlForm.NAME, c.getName());
		checkbox.putClientProperty(JXHtmlForm.VALUE, c.getValue());
		return checkbox;

	}

	private JComboBox createCombobox(Input input) {
		logInputInfo(input);
		Select sel = (Select) input;

		Set<Option> options = sel.getOptions();
		Option[] data = options.toArray(new Option[0]);
		_logger.info("\nSelect options:");
		_logger.info(options.toString());
		JComboBox combobox = new JComboBox(data);
		combobox
				.setPreferredSize(new Dimension(adjustInputSize(inputSize), 30));
		Option selected = sel.getSelected();
		combobox.putClientProperty(JXHtmlForm.NAME, sel.getName());
		combobox.putClientProperty(JXHtmlForm.VALUE, selected.getValue());
		combobox.setSelectedItem(selected);

		return combobox;

	}

	private void createFormMessage() {
		if (getFormMessage() == null)
			return;
		String msg = getFormMessage();
		if (msg == null)
			return;
		if (formMessage == null)
			formMessage = new JLabel();
		setFormMessage(msg);
		layout.row().grid().add(formMessage);

	}

	private JLabel createLabel(LabelInput labels, JComponent comp, String id) {
		String lblTxt = labels.getLabels().get(id);
		if (lblTxt == null)
			return new JLabel();
		JLabel lbl = new JLabel(lblTxt);
		lbl.setLabelFor(comp);

		return lbl;
	}

	private JScrollPane createListOption(Input input) {
		logInputInfo(input);
		XSelect sel = (XSelect) input;
		Set<Option> options = sel.getOptions();
		Option[] data = options.toArray(new Option[0]);
		_logger.info("\nMultiple Select options:");
		_logger.info(options.toString());
		DefaultListModel lItems = new DefaultListModel();

		for (int k = 0; k < data.length; k++)
			lItems.addElement(data[k]);

		JList list = new JList(lItems);
		list.setSelectionMode(ListSelectionModel.MULTIPLE_INTERVAL_SELECTION);
		list.setLayoutOrientation(JList.VERTICAL);
		List<Option> selected = sel.getSelectedValues();
		sel.setSelectedValues(selected);
		int[] indices = sel.getSelectedIndices();
		list.setSelectedIndices(indices);
		// _logger.info(sel.getSelectedValues());
		JScrollPane sp = new JScrollPane(list,
				ScrollPaneConstants.VERTICAL_SCROLLBAR_AS_NEEDED,
				ScrollPaneConstants.HORIZONTAL_SCROLLBAR_AS_NEEDED);
		sp.setPreferredSize(new Dimension(adjustInputSize(inputSize), 70));
		sp.putClientProperty(JXHtmlForm.NAME, sel.getName());
		sp.putClientProperty(JXHtmlForm.VALUE, sel.getSelectedValues());

		sp.validate();
		return sp;
	}

	private JPasswordField createPasswordField(Input input) {
		logInputInfo(input);
		Text t = (Text) input;
		_logger.info("input password value: " + t.getValue());
		Integer size = Integer.parseInt(t.getSize());
		inputSize = size.intValue();
		_logger.info("input password size: " + size.intValue());
		JPasswordField pass = new JPasswordField();
		pass.setColumns(adjustInputSize(size.intValue()));
		pass.setPreferredSize(new Dimension(pass.getColumns(), 25));
		pass.putClientProperty(JXHtmlForm.NAME, t.getName());
		pass.putClientProperty(JXHtmlForm.VALUE, t.getValue());
		return pass;

	}

	private JRadioButton createRadioButton(Input input) {
		logInputInfo(input);
		RadioInput r = (RadioInput) input;
		_logger.info("radio input value: " + r.getValue());
		JRadioButton rb = new JRadioButton(r.getValue(), r.isChecked());
		radioBG.add(rb);
		rb.putClientProperty(JXHtmlForm.NAME, r.getName());
		rb.putClientProperty(JXHtmlForm.VALUE, r.getValue());

		return rb;

	}

	private JScrollPane createTextArea(Input input) {
		logInputInfo(input);
		TextArea t = (TextArea) input;
		_logger.info("input textarea value: " + t.getValue());
		Integer rows = Integer.parseInt(t.getRows());
		_logger.info("input textarea rows: " + rows.intValue());
		Integer cols = Integer.parseInt(t.getCols());
		_logger.info("input textarea cols: " + cols.intValue());
		JTextArea ta = new JTextArea();
		ta.setRows(rows);
		ta.setColumns(adjustInputSize(cols.intValue()));
		ta.setAutoscrolls(true);
		JScrollPane sp = new JScrollPane(ta,
				ScrollPaneConstants.VERTICAL_SCROLLBAR_AS_NEEDED,
				ScrollPaneConstants.HORIZONTAL_SCROLLBAR_AS_NEEDED);
		sp.putClientProperty(JXHtmlForm.NAME, t.getName());
		sp.putClientProperty(JXHtmlForm.VALUE, t.getValue());

		sp.validate();
		return sp;
	}

	private JTextField createTextField(Input input) {
		logInputInfo(input);
		Text t = (Text) input;
		_logger.info("input text value: " + t.getValue());
		Integer size = Integer.parseInt(t.getSize());
		inputSize = size.intValue();
		_logger.info("input text size: " + size.intValue());
		JTextField tf = new JTextField();
		tf.setColumns(adjustInputSize(size.intValue()));
		tf.setPreferredSize(new Dimension(tf.getColumns(), 25));
		tf.putClientProperty(JXHtmlForm.NAME, t.getName());
		tf.putClientProperty(JXHtmlForm.VALUE, t.getValue());
		wInsets = tf.getInsets().left + tf.getInsets().right;
		System.out.println("text field wInsets=" + wInsets);
		hInsets = tf.getInsets().top + tf.getInsets().bottom;
		System.out.println("text field hInsets=" + hInsets);
		return tf;

	}

	public String getFormMessage() {
		String msg = null;
		try {

			Response response = api.getResponse();
			String html = response.getBody();
			// _logger.info(html);
			if (!html.startsWith("<?xml"))
				html = Forms.tidy(html);
			SimpleDocument dom = SimpleDocumentBuilder.simpleParse(html);
			Node n = dom.getElement("//p");
			if (n != null) {
				String val = n.getTextContent();
				Paragraph p = new Paragraph(n.getNodeName(), val);
				if (p != null)
					return p.getValue();
			}
			n = dom.getElement("//errormsg");
			if (n != null) {
				return n.getTextContent();
			}
			n = dom.getElement("//message");
			if (n != null) {
				return n.getTextContent();
			}

		} catch (Exception e) {
			throw new IllegalStateException(e);
		}

		return msg;
	}

	public List<Parameter> getFormParams() {
		List<org.jdesktop.swingx.html.form.Parameter> p = this.getJXForm()
				.getParameters();
		List<Parameter> params = new ArrayList<Parameter>();
		for (Iterator<org.jdesktop.swingx.html.form.Parameter> it = p
				.iterator(); it.hasNext();) {
			org.jdesktop.swingx.html.form.Parameter xparam = it.next();
			params.add(new Parameter(xparam.getName(), xparam.getValue()));
		}
		_logger.info("jxform params list:" + params.toString());
		return params;

	}

	public JXHtmlForm getJXForm() {
		if (jxform == null) {
			this.initJXForm();
			createFormMessage();
			Set<Input> comp = components.keySet();
			for (Iterator<Input> it = comp.iterator(); it.hasNext();)
				this.addComponent(it.next());

		}
		return jxform;

	}

	/**/

	public LabelInput getLabelInput() {
		return (LabelInput) form.getInput("label");
	}

	/**
	 * @return the sendButton
	 */
	public JButton getSendButton() {
		return sendButton;
	}

	public Title getTitle() {
		try {
			Title title = null;
			Response response = api.getResponse();
			String html = response.getBody();
			html = Forms.tidy(html);
			SimpleDocument dom = SimpleDocumentBuilder.simpleParse(html);
			for (Node n : dom.getElements("//title")) {
				if (n.getNodeName().equalsIgnoreCase("title")) {
					String val = n.getTextContent();
					title = new Title(n.getNodeName(), val);
				}
			}
			return title;
		} catch (Exception e) {
			throw new IllegalStateException(e);
		}

	}

	private void initJXForm() {
		jxform = new JXHtmlForm();
		layout = new DesignGridLayout(jxform);

		Input[] in = form.getInputs();
		_logger.info("number of html input fields=" + in.length);

		for (int i = 0; i < in.length; i++) {

			if (in[i] instanceof Select) {

				components.put(in[i], this.createCombobox(in[i]));

			} else if ((in[i] instanceof XSelect)) {
				components.put(in[i], this.createListOption(in[i]));

			} else if ((in[i] instanceof RadioInput)
					&& in[i].getType().equals("radio")) {
				components.put(in[i], this.createRadioButton(in[i]));

			} else if ((in[i] instanceof RadioInput)
					&& in[i].getType().equals("checkbox")) {
				components.put(in[i], this.createCheckBox(in[i]));

			} else if ((in[i] instanceof Text)
					&& in[i].getType().equals("text")) {
				components.put(in[i], this.createTextField(in[i]));

			} else if ((in[i] instanceof Text)
					&& in[i].getType().equals("password")) {
				components.put(in[i], this.createPasswordField(in[i]));

			} else if ((in[i] instanceof TextArea)
					&& in[i].getType().equals("textarea")) {

				components.put(in[i], this.createTextArea(in[i]));

			} else if (in[i].getType().equals("hidden")) {
				JLabel l = new JLabel();
				l.setVisible(false);
				l.putClientProperty(JXHtmlForm.NAME, in[i].getName());
				l.putClientProperty(JXHtmlForm.VALUE, in[i].getValue());
				layout.row().grid(l);

			} else if (in[i].getType().equals("submit")) {
				sendButton = this.createButton(in[i]);
				components.put(in[i], sendButton);

			}
		}

	}

	private String logInputInfo(Input input) {
		String id = input.getId() == null ? "null" : input.getId();
		String label = getLabelInput().getLabels().get(id) == null ? "null"
				: getLabelInput().getLabels().get(id);
		_logger.info(" type=" + input.getType() + " name=" + input.getName()
				+ " label=" + label + " id=" + id);
		return id;
	}

	/**
	 * @param formMessage
	 *            the formMessage to set
	 */
	public void setFormMessage(String formMessage) {
		this.formMessage.setText("<html>" + formMessage + "</html>");
	}

}   

[Ulf Dittmer]

Umm ... 750 lines of code? I don't think too many people here will be willing to go through that (not without taking a cut of the money you make for working on this anyway )

[Yahya Elyasse]

Ulf Dittmer wrote:Umm ... 750 lines of code? I don't think too many people here will be willing to go through that (not without taking a cut of the money you make for working on this anyway )

some people asked me to show them the code..so i paste this long code. if you find it inconvienient posting long code i can remove it no problem.

[Ulf Dittmer]

It's fine, I just don't think people will take the time to try to understand it.

If someone asks for code, they don't mean 750 lines of it ... maybe 50.

[Rob Spoor]

Read through it all

I did notice you hardly ever cache results. A few examples:

private void createFormMessage() {
    if (getFormMessage() == null)
        return;
    String msg = getFormMessage();
    if (msg == null)
        return;
    if (formMessage == null)
        formMessage = new JLabel();
    setFormMessage(msg);
    layout.row().grid().add(formMessage);
}

getFormMessage() retrieves the response body (from the URL directly, or cached, can't tell), and parses the entire body each time its called. If getFormMessage() returns a non-null value, you do this twice. For every call to getFormMessage(), that could be an entire parse that's completely unnecessary if you cache getFormMessage() before the firs null-check.

Then, in getTitle(), you get the response body and parse yet again. Also:

for (Node n : dom.getElements("//title")) {
    if (n.getNodeName().equalsIgnoreCase("title")) {
        String val = n.getTextContent();
        title = new Title(n.getNodeName(), val);
    }
}
return title;

Do you expect multiple titles, and then want to return the last one? If not, return title as soon as you set it. Because this way, you are going through all elements, even if you've found the title within the first few iterations.

public LabelInput getLabelInput() {
    return (LabelInput) form.getInput("label");
}

private String logInputInfo(Input input) {
    String id = input.getId() == null ? "null" : input.getId();
    String label = getLabelInput().getLabels().get(id) == null ? "null"
                   : getLabelInput().getLabels().get(id);
    _logger.info(" type=" + input.getType() + " name=" + input.getName()
                 + " label=" + label + " id=" + id);
    return id;
}

You call logInputInfo several times. Each time, regardless of the logger level, you possibly call input.getId() and getLabelInput().getLabels().get(id) twice. I have no idea how expensive those two methods are, but it really is unnecessary. You should change it to something like this:

private String logInputInfo(Input input) {
    String id = String.valueOf(input.getId()); // String.valueOf(null) returns "null"
    if (_logger.isLoggable(Level.INFO)) // using java.util.logging; _logger.isEnabledFor(Level.INFO) for Log4J
    {
        String label = String.valueOf(getLabelInput().getLabels().get(id)); // likewise
        // also, can't you cache the result of getLabelInput() and/or getLabelInput().getLabels() ?
        _logger.info(" type=" + input.getType() + " name=" + input.getName()
                     + " label=" + label + " id=" + id);
    }
    return id;
}

Those are just a few examples I noticed, but there are more.

Also, you should really, really consider caching more objects. I already mentioned getLabelInput() and/or getLabelInput().getLabels(), but also api.getResponse(), api.getResponse().getBody() (after you've tidied it), the parsed document, etc.

If you don't want to initialize these each time, you can lazy initialize them. For instance:

private transient Response response;
private transient String html;
private transient SimpleDocument dom;
private transient LabelInput labelInput;
private transient Map<String, String> labelInputLabels;

private Response getResponse()
{
    if (response == null)
    {
        response = api.getResponse();
    }
    return response;
}

private String getHTML()
{
    if (html == null)
    {
        html = Forms.tidy(getResponse().getBody()); // lazy initialization method called
    }
    return html;
}

private SimpleDocument getDocument()
{
    if (dom == null)
    {
        dom = SimpleDocumentBuilder.simpleParse(getHTML()); // lazy initialization method called
    }
    return dom;
}

public LabelInput getLabelInput()
{
    if (labelInput == null)
    {
        labelInput = form.getInput("label");
    }
    return labelInput;
}

private Map<String, String> getLabelInputLabels()
{
    if (labelInputLabels == null)
    {
        labelInputLabels = getLabelInput().getLabels();
    }
    return labelInputLabels;
}

[Yahya Elyasse]

Rob,
almost all methods are called once . I build form once and don't call most of methods twice. so i didn't considered caching results

[Rob Spoor]

You should consider caching at least the SimpleDocument; tidying and parsing is probably the most expensive task, and you're calling it at least twice - once in getFormMessage() and once in getTitle().

[Yahya Elyasse]

Rob Prime wrote:You should consider caching at least the SimpleDocument; tidying and parsing is probably the most expensive task, and you're calling it at least twice - once in getFormMessage() and once in getTitle().

hmm I think you are right. i noticed that the tidy(html) method is called many times and that's a quit expensive method because it processes the whole html stream.
So yes thanks for mentioning that.I'll try changing to your proposed solution.
BTW: do you think that parsing html and caching html Inputs should be done separately first ,then after that i can go through html Inputs array and build & render corresponding swing components? won't this hurt much performance? I can't see an elegant way to do parsing and rendering simultaneously. do you think i should plug rendering code inside Forms class that does the html parsing? that will produce inelegant code i believe.any Ideas?

thanks.

[Yahya Elyasse]

Thanks Rob,
I did the caching changes and now the rendering speed is much better (no more than 4 secs).
However i feel HTML parsing still takes longer time (i'm said it took 5 secs) but the progress let me feel it takes longer. could it because of the logging logic? because for debugging purposes i'm logging all html stream ect..
in class Forms.java above can you detect some inefficient parsing code i might have done?

thanks

[Rob Spoor]

Nope. The only thing you could remove is all the "catch all exceptions and wrap them in an IllegalStateException" - in the end, all the getFormXXX methods call getForm(SimpleDocument, String, String), and that already catches all exceptions and wraps them in an IllegalStateException. So what will be happening is the following:
SomeException is thrown
It is wrapped in an IllegalStateException
That is wrapped in an IllegalStateException
That is wrapped in an IllegalStateException
...

So except for that one method, you can remove all the try-catch blocks. Not that that will cause much improvement in speed, but it will help you in debugging should an exception occur.

[Yahya Elyasse]

Thank you Rob. you helped me much optimizing my code performance.
I still need to investigate the html parsing code and make it work faster.
any Ideas guys how to speed html form parsing ? are there any Java libraries out there for parsing html forms quickly and returning html Form input fields?

thanks

[Ulf Dittmer]

How is HTML parsing done now - are you using a library? I might convert it to XML (using something like Neko or TagSoup), and then run a SAX parser over it, or use a library like HtmlUnit.

[Yahya Elyasse]

Ulf Dittmer wrote:How is HTML parsing done now - are you using a library? I might convert it to XML (using something like Neko or TagSoup), and then run a SAX parser over it, or use a library like HtmlUnit.

I'm only using the java DOM parser . I treat the html stream like xml and parse it using a DOM parser.
I'm not sure if there exits some libs for faster html parsing and that collect html form input fields. you mentioned some libraries i will definitely take a look at them.
thanks.