[Dltk-dev] javascript parsing bugs and performance issues

["Johan Compagner" <jcompagner@xxxxxxxxx>]

Hi,

 

until now we didnt really notice or encouter it but when we tested a large project with many many js files

i got 2 problems, first was a pretty serious one because that caused a stack overflow

because a org.eclipse.dltk.internal._javascript_.typeinference.TransparanetRef had a OrReference as its evaluateReference

but that OrRefernce had for its second field again that TransparantRef instance.

So setLocal call resulted in a big stack overflow

 

When i was debuggin that i did see that there where so many nested OrReferences (and the special OrReferenceWriteSecond who can explain to me exactly when that is used for what?)

also it was not just nested of constantly different instances but just as with the TransparantRef instances where nested (but didn't really cause a stack overflow because the where in different paths)

The result was that call getChilds() on an or then it was possible that deep down on the one field it encounters exactly the same piece of tree then in the second field.

 

I got pretty much all those fixed by changing HostCollection.add() method

first it was this:

 

  public void add(String key, IReference ref) {
  if (ref == null)
   throw new IllegalArgumentException();
  Object object = reference.get(key);
  if (object == null) {
   reference.put(key, ref);
  } else {
   ref = new OrReference((IReference) object, ref);
   
   reference.put(key, ref);
  }
 }

 

i changed it to

 

public void add(String key, IReference ref) {
  if (ref == null)
   throw new IllegalArgumentException();
  Object object = reference.get(key);
  if (object == null) {
   reference.put(key, ref);
  } else if (object != ref) {
   if (ref instanceof OrReference)
   {
    // Test if those already are referencing each other.
    if ( ((OrReference)ref)._one_ == object)
    {
     reference.put(key, ref);
     return;
    }
   }
   else if (ref instanceof TransparentRef)
   {
    if (((TransparentRef)ref).evaluateReference == object)
    {
     reference.put(key, ref);
     return;
    }
   }

   ref = new OrReference((IReference) object, ref);
   
   reference.put(key, ref);
  }
 }

 

First i test there if object that is gotten from the reference map is not already the same as the given reference (creating an or around 2 the same objects seems silly)

That piece doesn't really matter

 

the second was that i test if the given ref was an OrReference then i test of the one field is already the same as the given object

(somehow the second field was never a problem)

If that is the case then we already have an Or for that object. So why wrap it again? Except that the ref that i could get could be a OrReferenceWriteSecond instead of just an OrReference

so now i dont have just that given object wrapped in an OrReference as the one field and also as a WriteSecond as the one field but onlly as a write second.

The only thing that seems different is that if the WriteSecond would get a call setPrototype() that now one wouldnt get that, and or sets it on both...

But looking at that i cant really see the use of that.

 

the third thing is the Transparent check. If the reference is already the given object just use the transparant ref and dont wrap it again in an or.

 

Even with all this you stull could have 2 branches of trees that had the same object. So a quick patch of OrReference getChild itself:

 

  if (child == child2)
  {
   return child;
  }
  return new OrReference(child, child2);

i added the child==child2 check just as above in the add method. So that not many completely silly OrReferences are created.

 

This seems to fix the speed problem. It is much faster (but i still think it is not as fast as i would have liked)

It did fix the stack overflow problem

 

The thing is that i dont know if the instanceof Or in the add() method will have side effects or not?

 

johan