how to optimize this code (recursion and iteratorions)

[vinay basavanal]

I have written some code which will fetch contents from a resource which is actually stored in a tree format. Since it is in a tree format, there will be a parent-child relation and hence recursion.

I am facing lot of performance issue as tree is growing in size this particular piece of code takes up-to 25 sec which is very bad. This piece of code will basically read data stored in file system (just for example) each content has certain set of property which it has to read.

import java.util.List;
public class Links {
    private String nodeName;
    private List<Links> children;

    public List<Links> getChildren() {
        return children;
    }

    public void setChildren( List<Links> children ) {
        this.children = children;
    }

    public String getNodeName(){
        return nodeName;
    }

    public void setNodeName( String nodeName ){
        this.nodeName = nodeName;
    }
}

package menu;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

public class Utility {
    /*
     * IN this class NavModel,CModel,CNode are some dummy classes which help us read contents form some resources which
     * are stored in dummy format as Example of tree format stored data below is the example tree child1 child2 child3
     * child3-1 child:q child:r child:a child3-2 child4
     */

    private static void populateLinks( NavModel navModel,
            ContentModel metaDataModel, Object objectNode, Links parent,
            boolean specialLinks ) throws IOException{
        try{
            List<Links> childLinks = new ArrayList<Links>();
                    // below code gets all the childrens of parent
            Iterator it = navModel.getChildren( objectNode );
            while( it.hasNext() ){
                NavNode node = (NavNode) it.next();
                ContentNode contentNode = node.getContentNode();

                Links links = new Links();

                MetaData data = metaDataModel.getMetaData( contentNode );

                Map<String, String> paramValues = new HashMap<String, String>();
                             // this particular piece of 
                             // code gets all the properties of content iterates and stores it in data structure
                Iterator metaDataIterator = data.getNames().iterator();
                while( metaDataIterator.hasNext() ){
                    String pagePropertyKey = metaDataIterator.next().toString();
                    String pagePropertyValue = (String) data
                    .getValue( pagePropertyKey );

                    if( pagePropertyKey.equalsIgnoreCase( "LINK_TYPE" ) ){
                        links.setLinkType( pagePropertyValue );
                    }
                    else if( pagePropertyKey.equalsIgnoreCase( "TCDID" ) ){
                        links.setTcmIdMap( pagePropertyValue );
                    }
                    else{
                        paramValues.put( pagePropertyKey, pagePropertyValue );
                    }
                }

                links.setParamValues( paramValues );

                if( specialLinks ){
                    links.setNodeName( links.getParamValues().get(
                    "APPLICATIONNAME" ) );
                }
                else{
                    links.setNodeName( contentNode.getTitle( new Locale(
                    "en_US" ) ) );
                }

                links.setDisplayName( setAppDispName( links.getNodeName() ) );
                childLinks.add( links );

                if( navModel.hasChildren( node ) ){
                                    // This is where recursion happens
                    populateLinks( navModel, metaDataModel, node, links,
                            specialLinks );
                }
            }
            parent.setChildren( childLinks );

        }
        catch( Exception e ){
        }
    }

  // THis is the method which calls the recursion function
   public static Links setupLinks( String categoryLinkName, String name ){
    Links categoryLinks = null;
    CModel contentModel = new CModel();
    NavModel navModel = new NavModel();
    categoryLinks = Utility.createCategoryLinks( categoryLinkName );
    Object objectNode = contentModel.getLocator().findByUniqueName( name );
    if( objectNode != null ){
        if( navModel.hasChildren( objectNode ) ){
            populateLinks( navModel, objectNode, categoryLinks );
        }
    }
}

    private static Object setAppDispName( String nodeName ){
        // fetch value from db
        return null;
    }    
}

I know code is not complete and may be in appropriate to review but I can't paste the whole of my code hence I hope you understand and review there is recursion and iteration any way I can better write this piece of code.

[fred rosenberger]

The first step in doing optimization is always to use a profiler to determine where it is slowing down.

Sure, you can look, and you can guess...but almost every time, you will guess wrong.

[Edward Harned]

Are you looking for a Fork/Join solution? F/J works well with recursion. Perhaps this article I wrote may help: A Java Fork-Join Conqueror

[Campbell Ritchie]

I had to edit the link in that last post because http appeared twice in it. It works nicely now

[vinay basavanal]

fred rosenberger wrote:The first step in doing optimization is always to use a profiler to determine where it is slowing down.

Sure, you can look, and you can guess...but almost every time, you will guess wrong.

populateLinks is the method which approximately takes about 70% of time i.e about 16 sec .It has recursion and iteration how can i optimize this.

[Martin Vashko]

vinay basavanal wrote:

        catch( Exception e ){
        }

You need to get rid of this. If something goes wrong, you won't even know about it.

It might even be the reason for bad performance: it is possible that your code throws a ton of exceptions, but you don't know about them. Since handling an exception is fairly expensive, having lots of them could easily slow the program down considerably (apart from giving wrong results because of the ignored exceptions, of course). This is just a guess. I don't know if your code throws any exceptions. But you don't know either

I would also try to avoid creating new locale again and again. In your case you should probably just use Locale.US.

[Winston Gutkowski]

vinay basavanal wrote:I have written some code which will fetch contents from a resource which is actually stored in a tree format. Since it is in a tree format, there will be a parent-child relation and hence recursion.
I am facing lot of performance issue as tree is growing in size this particular piece of code takes up-to 25 sec which is very bad. This piece of code will basically read data stored in file system (just for example) each content has certain set of property which it has to read.

Well, I can't be absolutely sure, but that seems to be an awful lot of procedure to convert something that, presumably, is already in a pseudo-tree format.

My suggestion: StopCoding (←click) and write down exactly how this mapping is supposed to work (ie, what you're trying to do).

I think, if it was me, I'd also make my Links class more atomic; perhaps something like:

public class Links {
    private final String nodeName;
    private final List<Links> children;

    public Links(String name, int numchildren) {
        this.nodeName = name;
        this.children = new ArrayList<Links>(numChildren);
    }

    public Links(String name) {
        this(name, 16);
    }

    public void addChild(Links child) {
        return children.add(child);
    }

   // other methods
   ...
}

which would allow you to build up each link incrementally.

I also worry a bit that the link doesn't contain a value other than its name (unless the name is the value). In general, node structures are used to store values in tree form and so generally have the form Node<T> (where T is the type of value stored), whereas all your class seems to do is store other nodes.

But it's quite possible I'm missing something.

Winston