Comment 1 Thomas Hallgren 2011-05-03 01:45:05 EDT

Perhaps an easy replacement would be something like:

public class Util {
  private static WeakHashMap<String,WeakReference<String>> strings;

  public static String weakIntern(String str) {
    synchronized(strings) {
       WeakReference<String> wr = strings.get(str);
       if(s != null) {
          String found = s.get();
          if(found != null)
             return found;
       }
       strings.put(str, new WeakReference<String>(str));
       return str;
    }
  }
}

Then replace all calls to str.intern() with Util.weakIntern(str)

This preserves a String lightweight pattern while still making them eligible for GC. I honestly thought that was what the JVM did in String.intern(). Obviously, it doesn't.

Comment 2 John Arthorne 2011-05-03 09:06:33 EDT

Interning definitely consumes more perm gen space. Dean and I both did measurements and found the overall memory consumption was down significantly, but perm gen usage did overall increase.

Interned strings *are* garbage collected in both Oracle and IBM JVMs. This is a bit hard to validate in p2 because repositories are held via weak reference.

273 repositories is a *lot* of data! I wonder what the memory usage would have been without our switching to interning. I suspect either way you would need to increase memory allocation for a case like that (heap, perm gen, or both).

Comment 3 John Arthorne 2011-05-03 09:11:49 EDT

Here's a quick test that demonstrates interned strings are garbage collected. If you run this you see equal interned strings have different identity after a gc. If you comment out the gc you will see the strings are the same.

public class A {
	public static void main(String[] arguments) {
		for (int i = 0; i < 100; i++) {
			createString();
			System.gc();
		}
	}
	private static void createString() {
		String s = Integer.toString(123456, 2).intern();
		System.out.println(System.identityHashCode(s));
	}
}

Comment 4 Thomas Hallgren 2011-05-05 03:25:09 EDT

Ian, what JVM did you use? What version?

Comment 5 Ian Bull 2011-05-05 12:01:13 EDT

(In reply to comment #4)
> Ian, what JVM did you use? What version?

I'm using the Sun / Oracle VM (1.6).  

As for whether Strings (or other Perm Gen) items are GCd, I need to apologize. I was under the impression that things in PermGen were not ever removed, but this was clearly a misunderstanding I had.

As for running with more memory. Yes, when loading close to 300 repos I had to increase the memory size (I think I gave the VM close to 5Gb) of space.  With Eclipse 3.6 there was no need to increase the PermGen size, this obviously changed with Eclipse 3.7.

I have no problem allocating more PermGen space, and if the string are collected (at some point) then I think using string#intern() as a string pool mechanism is perfectly fine.

I'm going to close this bug as WONTFIX. We may need to educate some people about the new memory requirements (if they are loading lots of data).

Thanks everyone!

Comment 6 Jeff McAffer 2011-05-05 16:53:13 EDT

yes, your last point about education is right on.  Most users don't know permgen from heap from, ...  all they know is "Eclipse crashes and it didn't used to".  Perhaps the default permgen space should be increased in the EPP packages? It will be interesting to try this out in the larger packages that were already bumping up against the permgen limits.  p2 might just push them over the edge.

Comment 7 John Arthorne 2011-05-06 15:52:56 EDT

Remy had a large install that contained Eclipse SDK 4.1 + all of Indigo. I asked him to perform the following test:

- Open every view
- Enable every p2 repository (about 30, plus the 4.1 I-build repository repo that has 23 children)
- Check for updates

This took a very long time, but blew the heap before running out of Perm Gen. When check for updates started, Perm Gen was at 170MB. It peaked at 201MB during check for updates. The max is 256MB.

I suspect people will only see this if they dramatically increase their heap size without increasing the perm gen size. Even then, it would take quite a large scenario to make it fail.

Comment 8 Ian Bull 2011-05-06 16:27:16 EDT

(In reply to comment #7)
Thanks John (and Remy).

Also, IIRC Eclipse (the equinox launcher) increases the PermGen space.  In my case I was running a RAP application (on Tomcat), so I had the default VM PermGen size + a *really* big heap.