Split gnu/javax/swing/text/html/parser/HTML_401F.java (Was [vta] Add chains from referenced VALUEs to DVs that reference them)

Split gnu/javax/swing/text/html/parser/HTML_401F.java (Was [vta] Add chains from referenced VALUEs to DVs that reference them)

[Andrew Haley]

Jakub Jelinek wrote:
> On Tue, Jun 30, 2009 at 03:50:11PM +0100, Andrew Haley wrote:
>> Gerald Pfeifer wrote:
>>> On Tue, 30 Jun 2009, Jakub Jelinek wrote:
>>>> Alternatively we could split the huge HTML_401F.java function into
>>>> say 4 smaller ones.
>>> Pleeeeeease! :-)  This one has been causing troubles beyond the context
>>> of just vta (where, for example, on some system one needs to boot with
>>> a special kernel option to provide sufficient amounts of memory to GCJ).

>> I can split the function, but papering over the problem by splitting
>> one function won't make the problem go away for others who use gcc.
>> It'll just mean that gcc developers don't notice it.
> 
> Sure, that's why I've spent last 2 weeks on var-tracking.c improvements
> for this exact testcase.  The question just is, if we really need to include
> so huge testcases as part of everybody's daily bootstrap/regtest cycle, or
> if it is sufficient to keep such testcases on the side, for automated
> testers that track their compile time/memory usage and nag us if we regress
> too much on it.
> 
>> The big function is about 250k lines of GIMPLE.  jc1 uses about 474m of
>> RAM at -O2 on a 64-bit system, 414m at -O1, 536m at -O0.  On what class
>> of machines are you trying to build this?
> 
> From var-tracking POV, especially on VTA branch, the main problem is
> that this function has 10000 basic blocks and on VTA needs to track over
> 15000 of variables/VALUEs across all those bbs.  Vanilla VTA branch needs
> 2.9GB of memory and 25 minutes to compile this at -g -O2, with all the
> patches I've sent it needs just 1.6GB of memory and 8 minutes.
> 
>> Don't we have some sort of heuristic that says "this function is
>> freaking huge, don't do any expensive optimizations." ?
> 
> For var-tracking we just bail out on highly connected large cfgs:
>   if (n_basic_blocks > 500 && n_edges / n_basic_blocks >= 20)
>     return 0;
> 
> I haven't studied how exactly is --enable-java-maintainer-mode
> compiling the classes; if I just gcj -C HTML_401F.java on
> Fedora 11 (GCC 4.4.0, ecj 3.4.2), the compile time with patched
> VTA is only 4:53 with 1.5GB top memory usage, if I patch HTML_401F.java
> with the following patch, it compiles within 0:55 and maxes at 250MB.
> I have no idea whether it will work correctly (what to test it with)
> and whether it is or is not an ABI change.

It's not an ABI change.  This patch is OK iff accompanied by a comment
in the code that explains the problem.

Thanks,
Andrew.

Re: Split gnu/javax/swing/text/html/parser/HTML_401F.java (Was [vta] Add chains from referenced VALUEs to DVs that reference them)

[Jakub Jelinek]

[-- Attachment #1: Type: text/plain, Size: 5047 bytes --]

On Wed, Jul 01, 2009 at 01:37:23PM +0100, Andrew Haley wrote:
> It's not an ABI change.  This patch is OK iff accompanied by a comment
> in the code that explains the problem.

Thanks, here is what I've committed.
In doing so, I had to --enable-java-maintainer-mode to update the class
file, which in addition to changing the HTML_401F* files changed a lot of
headers and a handful of classes as well, see attached second patch.
But at least the header changes look wrong, see e.g.
- namespace namespace$
+ namespace namespace
changes in the headers, I guess related to recent keyword handling changes.
Dunno if that is because I had a wrong gcjh or if there is some recent issue in the
tools.

I've only committed the HTML_401F* files, the second patch is just FYI.

Index: classpath/ChangeLog.gcj
===================================================================
--- classpath/ChangeLog.gcj	(revision 149147)
+++ classpath/ChangeLog.gcj	(working copy)
@@ -1,3 +1,10 @@
+2009-07-01  Jakub Jelinek  <jakub@redhat.com>
+
+	* gnu/javax/swing/text/html/parser/HTML_401F.java (defineElements):
+	Split this huge method into...
+	(defineElements1, defineElements2, defineElements3, defineElements4,
+	defineElements5, defineElements6): ... these smaller methods.
+
 2009-06-16  Matthias Klose  <doko@ubuntu.com>
 
 	* tools/gnu/classpath/tools/gjdoc/Main.java (getGjdocVersion): Use
Index: classpath/gnu/javax/swing/text/html/parser/HTML_401F.java
===================================================================
--- classpath/gnu/javax/swing/text/html/parser/HTML_401F.java	(revision 149147)
+++ classpath/gnu/javax/swing/text/html/parser/HTML_401F.java	(working copy)
@@ -379,6 +379,24 @@ public class HTML_401F
    */
   protected void defineElements()
   {
+    /* Define the elements.  This used to be one huge method, which
+       unfortunately took too long to compile and consumed
+       too much memory while compiling it.  While it can serve as
+       a good stress test for gcj, it is better to split it up
+       to save time and memory used during GCC bootstrap.  */
+    defineElements1();
+    defineElements2();
+    defineElements3();
+    defineElements4();
+    defineElements5();
+    defineElements6();
+  }
+
+  /**
+   * Define first sixth of elements of this DTD.
+   */
+  private void defineElements1()
+  {
     /* Define the elements. */
       defElement(PCDATA, 0, false, false, null, NONE, NONE,
         new AttributeList[ 0 ]);
@@ -885,6 +903,15 @@ public class HTML_401F
           0, IMPLIED)
       }
     );
+
+  }
+
+  /**
+   * Define second sixth of elements of this DTD.
+   */
+  private void defineElements2()
+  {
+    /* Define the elements. */
       defElement(CENTER, 0, false, false, null,
       NONE
       ,
@@ -1373,6 +1400,15 @@ public class HTML_401F
         attr(ONKEYUP, null, null, 0, IMPLIED)
       }
     );
+
+  }
+
+  /**
+   * Define third sixth of elements of this DTD.
+   */
+  private void defineElements3()
+  {
+    /* Define the elements. */
       defElement(FONT, 0, false, false, null,
       NONE
       ,
@@ -1861,6 +1897,15 @@ public class HTML_401F
         attr(VSPACE, null, null, 0, IMPLIED)
       }
     );
+
+  }
+
+  /**
+   * Define fourth sixth of elements of this DTD.
+   */
+  private void defineElements4()
+  {
+    /* Define the elements. */
       defElement(INPUT, EMPTY, false, true, null,
       NONE
       ,
@@ -2357,6 +2402,15 @@ public class HTML_401F
         attr(VSPACE, null, null, 0, IMPLIED)
       }
     );
+
+  }
+
+  /**
+   * Define fifth sixth of elements of this DTD.
+   */
+  private void defineElements5()
+  {
+    /* Define the elements. */
       defElement(OL, 0, false, false, createListModel(),
       NONE
       ,
@@ -2859,6 +2913,15 @@ public class HTML_401F
         attr(ONKEYUP, null, null, 0, IMPLIED)
       }
     );
+
+  }
+
+  /**
+   * Define last sixth of elements of this DTD.
+   */
+  private void defineElements6()
+  {
+    /* Define the elements. */
       defElement(SUP, 0, false, false, null,
       NONE
       ,
Index: classpath/lib/gnu/javax/swing/text/html/parser/HTML_401F.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: gnu/javax/swing/text/html/parser/HTML_401F.h
===================================================================
--- gnu/javax/swing/text/html/parser/HTML_401F.h	(revision 149147)
+++ gnu/javax/swing/text/html/parser/HTML_401F.h	(working copy)
@@ -58,6 +58,14 @@ public:
   static ::javax::swing::text::html::parser::DTD * getInstance();
 public: // actually protected
   virtual void defineElements();
+private:
+  void defineElements1();
+  void defineElements2();
+  void defineElements3();
+  void defineElements4();
+  void defineElements5();
+  void defineElements6();
+public: // actually protected
   virtual void defineEntities();
   virtual ::javax::swing::text::html::parser::ContentModel * model(::java::lang::String *);
 private:

	Jakub

[-- Attachment #2: P.bz2 --]
[-- Type: application/x-bzip2, Size: 7747 bytes --]

Re: Split gnu/javax/swing/text/html/parser/HTML_401F.java (Was [vta] Add chains from referenced VALUEs to DVs that reference them)

[Andrew Haley]

Jakub Jelinek wrote:
> On Wed, Jul 01, 2009 at 01:37:23PM +0100, Andrew Haley wrote:
>> It's not an ABI change.  This patch is OK iff accompanied by a comment
>> in the code that explains the problem.
> 
> Thanks, here is what I've committed.
> In doing so, I had to --enable-java-maintainer-mode to update the class
> file, which in addition to changing the HTML_401F* files changed a lot of
> headers and a handful of classes as well, see attached second patch.
> But at least the header changes look wrong, see e.g.
> - namespace namespace$
> + namespace namespace
> changes in the headers, I guess related to recent keyword handling changes.
> Dunno if that is because I had a wrong gcjh or if there is some recent issue in the
> tools.

I changed gcjh two days ago, so it is very likely you don't have the new
version.

I can't see any attachments.

Andrew.

Re: Split gnu/javax/swing/text/html/parser/HTML_401F.java

[Gerald Pfeifer]

On Wed, 1 Jul 2009, Andrew Haley wrote:
>> I haven't studied how exactly is --enable-java-maintainer-mode
>> compiling the classes; if I just gcj -C HTML_401F.java on
>> Fedora 11 (GCC 4.4.0, ecj 3.4.2), the compile time with patched
>> VTA is only 4:53 with 1.5GB top memory usage, if I patch HTML_401F.java
>> with the following patch, it compiles within 0:55 and maxes at 250MB.

That's quite a nice improvement.  HTML_401F.java has been causing 
troubles for many years, and splitting it really helps, for example
building on (virtual) machines with not so much main memory or in
limited settings where there is a process limit for 512MB.

> It's not an ABI change.  This patch is OK iff accompanied by a
> comment in the code that explains the problem.

I believe the patch has not made it into GCC Subversion yet.  Are
the two of you still planning to apply it?

Gerald

Re: Split gnu/javax/swing/text/html/parser/HTML_401F.java

[Jakub Jelinek]

On Sun, Jul 12, 2009 at 10:27:07PM +0200, Gerald Pfeifer wrote:
> On Wed, 1 Jul 2009, Andrew Haley wrote:
> >> I haven't studied how exactly is --enable-java-maintainer-mode
> >> compiling the classes; if I just gcj -C HTML_401F.java on
> >> Fedora 11 (GCC 4.4.0, ecj 3.4.2), the compile time with patched
> >> VTA is only 4:53 with 1.5GB top memory usage, if I patch HTML_401F.java
> >> with the following patch, it compiles within 0:55 and maxes at 250MB.
> 
> That's quite a nice improvement.  HTML_401F.java has been causing 
> troubles for many years, and splitting it really helps, for example
> building on (virtual) machines with not so much main memory or in
> limited settings where there is a process limit for 512MB.
> 
> > It's not an ABI change.  This patch is OK iff accompanied by a
> > comment in the code that explains the problem.
> 
> I believe the patch has not made it into GCC Subversion yet.  Are
> the two of you still planning to apply it?

See http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149148

	Jakub

Re: Split gnu/javax/swing/text/html/parser/HTML_401F.java

[Gerald Pfeifer]

On Sun, 12 Jul 2009, Jakub Jelinek wrote:
> See http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149148

Oh, thanks!  And sorry for having missed this. :-(

Gerald

Re: [cp-patches] Split gnu/javax/swing/text/html/parser/HTML_401F.java

[Audrius Meskauskas]

On Sun, Jul 12, 2009 at 10:27:07PM +0200, Gerald Pfeifer wrote:

> > On Wed, 1 Jul 2009, Andrew Haley wrote:
>   
>>> > >> I haven't studied how exactly is --enable-java-maintainer-mode
>>> > >> compiling the classes; if I just gcj -C HTML_401F.java on
>>> > >> Fedora 11 (GCC 4.4.0, ecj 3.4.2), the compile time with patched
>>> > >> VTA is only 4:53 with 1.5GB top memory usage, if I patch HTML_401F.java
>>> > >> with the following patch, it compiles within 0:55 and maxes at 250MB.
>>>       
> > 
> > That's quite a nice improvement.  HTML_401F.java has been causing 
> > troubles for many years, and splitting it really helps, for example
> > building on (virtual) machines with not so much main memory or in
> > limited settings where there is a process limit for 512MB.
> > 
>   
>> > > It's not an ABI change.  This patch is OK iff accompanied by a
>> > > comment in the code that explains the problem.
>>     
> > 
> > I believe the patch has not made it into GCC Subversion yet.  Are
> > the two of you still planning to apply it?
>   

See http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149148

	Jakub





Masters, where is the beginning of this discussion and where is the 
proposed patch? I have received four messages about HTML_401F that look 
completely in the middle of the context. While it is great when somebody 
continues your work, I think it would make no harm for me to look into 
the patch on the class I once wrote.

Audrius Meskauskas

Re: [cp-patches] Split gnu/javax/swing/text/html/parser/HTML_401F.java

[Andrew John Hughes]

2009/7/14 Audrius Meskauskas <audriusa@bluewin.ch>:
> On Sun, Jul 12, 2009 at 10:27:07PM +0200, Gerald Pfeifer wrote:
>
>> > On Wed, 1 Jul 2009, Andrew Haley wrote:
>>
>>>>
>>>> > >> I haven't studied how exactly is --enable-java-maintainer-mode
>>>> > >> compiling the classes; if I just gcj -C HTML_401F.java on
>>>> > >> Fedora 11 (GCC 4.4.0, ecj 3.4.2), the compile time with patched
>>>> > >> VTA is only 4:53 with 1.5GB top memory usage, if I patch
>>>> > >> HTML_401F.java
>>>> > >> with the following patch, it compiles within 0:55 and maxes at
>>>> > >> 250MB.
>>>>
>>
>> > > That's quite a nice improvement.  HTML_401F.java has been causing >
>> > > troubles for many years, and splitting it really helps, for example
>> > building on (virtual) machines with not so much main memory or in
>> > limited settings where there is a process limit for 512MB.
>> >
>>>
>>> > > It's not an ABI change.  This patch is OK iff accompanied by a
>>> > > comment in the code that explains the problem.
>>>
>>
>> > > I believe the patch has not made it into GCC Subversion yet.  Are
>> > the two of you still planning to apply it?
>>
>
> See http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149148
>
>        Jakub
>
>
>
>
>
> Masters, where is the beginning of this discussion and where is the proposed
> patch? I have received four messages about HTML_401F that look completely in
> the middle of the context. While it is great when somebody continues your
> work, I think it would make no harm for me to look into the patch on the
> class I once wrote.
>
> Audrius Meskauskas
>
>

Audrius, the patch is visible from the link posted by Jakub:
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149148
It simply splits the method which defines the entities into five
separate methods to reduce load on the compiler.

Is this generally useful? If so, it should go into GNU Classpath
rather than just the downstream copy in GCJ.
-- 
Andrew :-)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net

PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8

Re: [cp-patches] Split gnu/javax/swing/text/html/parser/HTML_401F.java

[Andrew Haley]

On 07/28/2009 12:55 AM, Andrew John Hughes wrote:
> 2009/7/14 Audrius Meskauskas <audriusa@bluewin.ch>:
>> On Sun, Jul 12, 2009 at 10:27:07PM +0200, Gerald Pfeifer wrote:
>>
>>>> On Wed, 1 Jul 2009, Andrew Haley wrote:
>>>>>>>> I haven't studied how exactly is --enable-java-maintainer-mode
>>>>>>>> compiling the classes; if I just gcj -C HTML_401F.java on
>>>>>>>> Fedora 11 (GCC 4.4.0, ecj 3.4.2), the compile time with patched
>>>>>>>> VTA is only 4:53 with 1.5GB top memory usage, if I patch
>>>>>>>> HTML_401F.java
>>>>>>>> with the following patch, it compiles within 0:55 and maxes at
>>>>>>>> 250MB.
>>>>> That's quite a nice improvement.  HTML_401F.java has been causing >
>>>>> troubles for many years, and splitting it really helps, for example
>>>> building on (virtual) machines with not so much main memory or in
>>>> limited settings where there is a process limit for 512MB.
>>>>
>>>>
>>>>>> It's not an ABI change.  This patch is OK iff accompanied by a
>>>>>> comment in the code that explains the problem.
>>>>> I believe the patch has not made it into GCC Subversion yet.  Are
>>>> the two of you still planning to apply it?
>> See http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149148
>>
>>        Jakub
>>
>>
>>
>>
>>
>> Masters, where is the beginning of this discussion and where is the proposed
>> patch? I have received four messages about HTML_401F that look completely in
>> the middle of the context. While it is great when somebody continues your
>> work, I think it would make no harm for me to look into the patch on the
>> class I once wrote.
>>
>> Audrius Meskauskas
>>
>>
> 
> Audrius, the patch is visible from the link posted by Jakub:
> http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149148
> It simply splits the method which defines the entities into five
> separate methods to reduce load on the compiler.
> 
> Is this generally useful? If so, it should go into GNU Classpath
> rather than just the downstream copy in GCJ.

I think it should go into Classpath anyway: it doesn't hurt anything
and it reduces divergence with gcj.

Andrew.

Re: [cp-patches] Split gnu/javax/swing/text/html/parser/HTML_401F.java

[Audrius Meskauskas]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

This change is ok. It should have no any impact on the algorithm.

Audrius

Andrew Haley wrote:
> On 07/28/2009 12:55 AM, Andrew John Hughes wrote:
>> 2009/7/14 Audrius Meskauskas <audriusa@bluewin.ch>:
>>> On Sun, Jul 12, 2009 at 10:27:07PM +0200, Gerald Pfeifer wrote:
>>>
>>>>> On Wed, 1 Jul 2009, Andrew Haley wrote:
>>>>>>>>> I haven't studied how exactly is --enable-java-maintainer-mode
>>>>>>>>> compiling the classes; if I just gcj -C HTML_401F.java on
>>>>>>>>> Fedora 11 (GCC 4.4.0, ecj 3.4.2), the compile time with patched
>>>>>>>>> VTA is only 4:53 with 1.5GB top memory usage, if I patch
>>>>>>>>> HTML_401F.java
>>>>>>>>> with the following patch, it compiles within 0:55 and maxes at
>>>>>>>>> 250MB.
>>>>>> That's quite a nice improvement.  HTML_401F.java has been causing >
>>>>>> troubles for many years, and splitting it really helps, for example
>>>>> building on (virtual) machines with not so much main memory or in
>>>>> limited settings where there is a process limit for 512MB.
>>>>>
>>>>>
>>>>>>> It's not an ABI change.  This patch is OK iff accompanied by a
>>>>>>> comment in the code that explains the problem.
>>>>>> I believe the patch has not made it into GCC Subversion yet.  Are
>>>>> the two of you still planning to apply it?
>>> See http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149148
>>>
>>>        Jakub
>>>
>>>
>>>
>>>
>>>
>>> Masters, where is the beginning of this discussion and where is the proposed
>>> patch? I have received four messages about HTML_401F that look completely in
>>> the middle of the context. While it is great when somebody continues your
>>> work, I think it would make no harm for me to look into the patch on the
>>> class I once wrote.
>>>
>>> Audrius Meskauskas
>>>
>>>
>> Audrius, the patch is visible from the link posted by Jakub:
>> http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149148
>> It simply splits the method which defines the entities into five
>> separate methods to reduce load on the compiler.
>>
>> Is this generally useful? If so, it should go into GNU Classpath
>> rather than just the downstream copy in GCJ.
> 
> I think it should go into Classpath anyway: it doesn't hurt anything
> and it reduces divergence with gcj.
> 
> Andrew.
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkp2CIIACgkQYtLS/5wKz7AuDQCgi667yXx++8u7GREOBFli3sWm
9f0AnAgLktVYkhVz2PPFQ/u/mr5llrHl
=8iy1
-----END PGP SIGNATURE-----

Re: Split gnu/javax/swing/text/html/parser/HTML_401F.java

[Andrew Haley]

Gerald Pfeifer wrote:
> On Wed, 1 Jul 2009, Andrew Haley wrote:
>>> I haven't studied how exactly is --enable-java-maintainer-mode
>>> compiling the classes; if I just gcj -C HTML_401F.java on
>>> Fedora 11 (GCC 4.4.0, ecj 3.4.2), the compile time with patched
>>> VTA is only 4:53 with 1.5GB top memory usage, if I patch HTML_401F.java
>>> with the following patch, it compiles within 0:55 and maxes at 250MB.
> 
> That's quite a nice improvement.  HTML_401F.java has been causing 
> troubles for many years, and splitting it really helps, for example
> building on (virtual) machines with not so much main memory or in
> limited settings where there is a process limit for 512MB.
> 
>> It's not an ABI change.  This patch is OK iff accompanied by a
>> comment in the code that explains the problem.
> 
> I believe the patch has not made it into GCC Subversion yet.

I believe it has.

Andrew.