934359 - Encounter 'java.lang.OutOfMemoryError' error when compiling with command 'make -f client.mk'

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Cedric Wang]

I encountered the following error when compiling gecko with command 'make -f client.mk'

Exception in thread "main" java.lang.OutOfMemoryError: Java heap space
	at java.util.Arrays.copyOf(Arrays.java:2882)
	at java.lang.AbstractStringBuilder.expandCapacity(AbstractStringBuilder.java:100)
	at java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:572)
	at java.lang.StringBuilder.append(StringBuilder.java:203)
	at org.mozilla.gecko.annotationProcessors.utils.Utils.writeTypeSignature(Utils.java:248)
	at org.mozilla.gecko.annotationProcessors.utils.Utils.getTypeSignatureString(Utils.java:228)
	at org.mozilla.gecko.annotationProcessors.utils.AlphabeticMethodComparator.compare(AlphabeticMethodComparator.java:23)
	at org.mozilla.gecko.annotationProcessors.utils.AlphabeticMethodComparator.compare(AlphabeticMethodComparator.java:10)
	at java.util.Arrays.mergeSort(Arrays.java:1270)
	at java.util.Arrays.sort(Arrays.java:1210)
	at org.mozilla.gecko.annotationProcessors.utils.GeneratableEntryPointIterator.<init>(GeneratableEntryPointIterator.java:30)
	at org.mozilla.gecko.annotationProcessors.AnnotationProcessor.main(AnnotationProcessor.java:44)

This compile error is caused by the following code segment in 'gecko/build/annotationProcessors/utils/Utils.java'

int len = name.length();
        while (name.endsWith("[]")) {
            sb.append('[');
            name = name.substring(0, len - 2);
        }

The variable 'len' is not assigned the new length(len -2) at the end of the 'while', so when the variable 'name' is a string likes 'boolean[][]',there will be a endless loop.

Comment 1

[Chris Kitching [:ckitching]]

Attachment: Don't explode when faced with multi-dimensional arrays.

Whoops!

You appear to be exactly right. He's an patch doing exactly what you said.
Flagging the original reviewer of this code for a rubber-stamp.

Comment 2

[Cedric Wang]

Attachment: Solve endless loop problem in gecko/build/annotationProcessors/utils/Utils.java

Hi, Chris Kitching,could you help me review this patch?

Comment 3

[Chris Kitching [:ckitching]]

Comment on attachment 826646 [details] [diff] [review]
Solve endless loop problem in gecko/build/annotationProcessors/utils/Utils.java

Sorry. Beat you to it by a tiny bit there!

Still, your patch looks fine - probably not necessary to include "[PATCH]" at the beginning of the commit message.

It also occurs to me that my patch is actually sort of dumb - an extra function call? that's not sensible. Your approach will perform a little better, as well as being shorter. Let's do it your way.

Just tweak that commit message thing (I think there's sort of strict rules about everything starting with just "Bug XXXXX") and it looks fine to me!

Comment 4

[Chris Kitching [:ckitching]]

You could also perhaps formulate it as len -= 2?

Comment 5

[Chris Kitching [:ckitching]]

Attachment: Solve endless loop problem in gecko/build/annotationProcessors/utils/Utils.java

This seems to have stagnated for a while...

Rubberstamp, please!

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 833355 [details] [diff] [review]
Solve endless loop problem in gecko/build/annotationProcessors/utils/Utils.java

Review of attachment 833355 [details] [diff] [review]:
-----------------------------------------------------------------

Either use cedric's version or move the int to be local inside the loop (or just inline it). I don't like exiting the loop with len !〓 length() even though len isn't used after that. rs〓me with any of those.

Comment 7

[Chris Kitching [:ckitching]]

Attachment: Solve endless loop problem in gecko/build/annotationProcessors/utils/Utils.java

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> Either use cedric's version or move the int to be local inside the loop (or
> just inline it). I don't like exiting the loop with len != length() even
> though len isn't used after that. rs=me with any of those.

Amazing, I uploaded the wrong patch.

Yes, Cedric's approach is clearly far more sane than what I just uploaded. Subtraction! It's subtraction! How did I get subtraction wrong!? :P

Actual rubber-stamp this time, then we can add this to the ever-increasing queue of things to land once the tree opens.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 826646 [details] [diff] [review]
Solve endless loop problem in gecko/build/annotationProcessors/utils/Utils.java

This one has the right author.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/fx-team/rev/94d56e88f893

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/94d56e88f893