2 Replies Latest reply on Nov 17, 2006 6:57 PM by Manik Surtani

    TreeCacheProxyImpl.getChildren() ugliness

    Elias Ross Master


      I've been taking a look at the AOP collections, in particular the performance -- or obvious lack of.

      Under a profiler, I found out that List.size() works a bit more than probably necessary. It ends up calling, getChildren().size(), which does this sort of thing:

       public Set<Node> getChildren()
       {
       Map m = currentNode.getChildren();
       Set<Node> children = new HashSet<Node>(m.size());
       Iterator i = m.values().iterator();
       while (i.hasNext())
       {
       children.add(new TreeCacheProxyImpl(treeCache, (NodeImpl) i.next()));
       }
      
       return Collections.unmodifiableSet(children);
       }
      


      Is there any particular approach that's being taken to hopefully improve this method's performance?

      It seems fairly straightforward to simply have NodeImpl implement NodeSPI, but this requires the getChildren() return type to change.

        • 1. Re: TreeCacheProxyImpl.getChildren() ugliness
          Elias Ross Master

          Okay, not to crap on everyone... So that method (List.size()) was 20% of the runtime for whatever reason.

          As an experiment, I changed getChildren() to getChildrenMap() in NodeImpl, so I could mix in NodeSPI. Then I fixed up a few classes here and there. Life seemed good. This method seemed fixable...

          But then... I reran the tests, but they failed. Oops! I fixed some casts in the LockingInterceptor. Tests hang. Curious, I searched for any additional dependencies on TreeCacheProxyImpl. Eclipse found like 93 references (many casts) to TreeCacheProxyImpl. All those would have to be changed for me to go futher...

          My immediate reaction was: Fuck. 90 places in the code now depends directly on this class, I don't know how many are casts.

          The only refactoring that makes sense is to fix existing node classes, retrofit TreeCache with CacheSPI and get rid of this shit. There's just no way I'd run 2.0 seeing what's in the sausage now.

          And I don't see the value of the new API if it's just a ugly retrofit with worse performance. "-1" as they say...

          • 2. Re: TreeCacheProxyImpl.getChildren() ugliness
            Manik Surtani Master

            Hey there.

            Yeah, a lot of this is due to be sorted out now that 2.0.0 is in focus again. The TCPI stuff has been a bit of a hack as a temporary fix to get around making specific design decisions - such as methods like getChildren() and the bazillion casts. A lot of this goes away once we identify better what needs to be in the SPI interface. I don't expect there to be more than a couple of casts to TCPI, if at all, as this breaks the whole purpose of the interface.

            You are looking at incomplete code, stitched together to make the Cache interface workable. Once the internals are complete things will be a lot quicker.

            Cheers,
            Manik