logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stephen Pain" <stephen.p...@db.com>
Subject Re: cvs commit: logging-log4j/src/java/org/apache/log4j/chainsaw/help release-notes.html
Date Sat, 07 Aug 2004 00:11:47 GMT

Hi Paul,

Thanks for fixing the problem with loading huge events. Unfortunately, it's dramatically slowed
down the loading of an xml file - a 23MB file I have takes 20 seconds with the previous version,
and 140 seconds with the new version!

I've played around a bit with a few options resulting in the patch I've attached.  It fixes
the bug by checking for a null returned from decodeEvents(...) & improves the performance
by around 30-50% (depending on file size) compared to the original code (just by using a buffer
of 1000 lines rather than 100). I've checked the memory usage and it seemed to have no adverse
effect. In fact, the 1000 lines version used 2MB *less* memory overall than the 100 lines
version when loading the 23MB test file.

Since someone already did all the work to deal with partial events, it seems sensible to use
it (of course, increasing the buffer to 1000 lines probably makes it fairly pointless to do
the null checking - but better to be safe, since there's so little cost involved).

If you think the above is sensible could you commit this patch?

      (See attached file: xmldecoder_patch.txt)

Here's the patch copied & pasted, in case there's a problem with the attachment:

Index: XMLDecoder.java
===================================================================
RCS file: /home/cvspublic/logging-log4j/src/java/org/apache/log4j/xml/XMLDecoder.java,v
retrieving revision 1.21
diff -u -r1.21 XMLDecoder.java
--- XMLDecoder.java     4 Aug 2004 03:48:12 -0000     1.21
+++ XMLDecoder.java     6 Aug 2004 23:49:16 -0000
@@ -25,7 +25,6 @@
 import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Map;
 import java.util.Vector;

@@ -156,23 +155,16 @@
     Vector v = new Vector();

     String line = null;
+     Vector events = null;
     try {
-        /**
-         * Keep reading in the lines until we find the end of record line
-         *
-         * NOTE: this might chew a bit more memory than the previous implementation which
-         * read in line blocks of 100, but allows a HUGE event spread over 100+ lines to
be decode correctly.
-         */
         while ((line = reader.readLine()) != null) {
-            StringBuffer buffer = new StringBuffer(512);
-            while(line!=null && line.indexOf(RECORD_END)==-1) {
-                buffer.append(line).append("\n");
-                line = reader.readLine();
+            StringBuffer buffer = new StringBuffer(line);
+            for (int i = 0;i<1000;i++) {
+                buffer.append(reader.readLine());
             }
-            if(line!=null) {
-             buffer.append(line);
-            }
-            v.addAll(decodeEvents(buffer.toString()));
+                 events = decodeEvents(buffer.toString());
+            if (events != null)
+                 v.addAll(events);
         }
     } finally {
       partialEvent = null;
@@ -188,16 +180,10 @@
   }

   public Vector decodeEvents(String document) {
-    /**
-     * NOTE: due to the logic above, which should read in the string containing the WHOLE
event text,
-     * there should be no need to track partial events, but I have not changed the code just
in case....
-     *
-     * Paul Smith 4th August 2004
-     */
     if (document != null) {
       document = document.trim();
       if (document.equals("")) {
-        return new Vector();
+        return null;
       } else {
            String newDoc=null;
            String newPartialEvent=null;

----------------------------------------------------------------------------------------

Thanks,
Stephen



                                                                                         
                                             
                      psmith@apache.org                                                  
                                             
                                               To:       logging-log4j-cvs@apache.org    
                                             
                      04/08/2004 04:48         cc:                                       
                                             
                      Please respond to        Subject:  cvs commit: logging-log4j/src/java/org/apache/log4j/chainsaw/help
            
                      "Log4J Developers         release-notes.html                       
                                             
                      List"                                                              
                                             
                                                                                         
                                             
                                                                                         
                                             




psmith      2004/08/03 20:48:13

  Modified:    src/java/org/apache/log4j/chainsaw/version
                        VersionManager.java
               src/java/org/apache/log4j/chainsaw FileLoadAction.java
               src/java/org/apache/log4j/xml XMLDecoder.java
               src/java/org/apache/log4j/chainsaw/help release-notes.html
  Log:
  Changes for Bugs 30443 & 30444

  30443 - The FileLoadAction class ignored the user's cancel action and loaded the file anyway

  30444 - An XML file containing log4j events that were over 100 lines would not be imported
correctly and generate an NPE.   Changed the logic
  to keep reading in lines until it reached the end of record, or end of file before attempting
to decode the text.

  PR: 30443 30444

  Revision  Changes    Path
  1.12      +1 -1      logging-log4j/src/java/org/apache/log4j/chainsaw/version/VersionManager.java

  Index: VersionManager.java
  ===================================================================
  RCS file: /home/cvs/logging-log4j/src/java/org/apache/log4j/chainsaw/version/VersionManager.java,v
  retrieving revision 1.11
  retrieving revision 1.12
  diff -u -r1.11 -r1.12
  --- VersionManager.java            31 Jul 2004 07:23:57 -0000          1.11
  +++ VersionManager.java            4 Aug 2004 03:48:12 -0000           1.12
  @@ -10,7 +10,7 @@

       private static final VersionManager instance = new VersionManager();

  -    private static final String VERSION_INFO = "1.99.99 (31 July 2004)";
  +    private static final String VERSION_INFO = "1.99.99 (4 Aug 2004)";

       public static final VersionManager getInstance() {
           return instance;



  1.13      +5 -2      logging-log4j/src/java/org/apache/log4j/chainsaw/FileLoadAction.java

  Index: FileLoadAction.java
  ===================================================================
  RCS file: /home/cvs/logging-log4j/src/java/org/apache/log4j/chainsaw/FileLoadAction.java,v
  retrieving revision 1.12
  retrieving revision 1.13
  diff -u -r1.12 -r1.13
  --- FileLoadAction.java            27 May 2004 12:16:57 -0000          1.12
  +++ FileLoadAction.java            4 Aug 2004 03:48:12 -0000           1.13
  @@ -99,10 +99,13 @@
             }
           });

  -      chooser.showOpenDialog(parent);
  -
  +      int i = chooser.showOpenDialog(parent);
  +      if(i != JFileChooser.APPROVE_OPTION) {
  +       return;
  +      }
         File selectedFile = chooser.getSelectedFile();

  +
         try {
           url = selectedFile.toURL();
           name = selectedFile.getName();



  1.21      +21 -4     logging-log4j/src/java/org/apache/log4j/xml/XMLDecoder.java

  Index: XMLDecoder.java
  ===================================================================
  RCS file: /home/cvs/logging-log4j/src/java/org/apache/log4j/xml/XMLDecoder.java,v
  retrieving revision 1.20
  retrieving revision 1.21
  diff -u -r1.20 -r1.21
  --- XMLDecoder.java          9 Jul 2004 14:41:06 -0000           1.20
  +++ XMLDecoder.java          4 Aug 2004 03:48:12 -0000           1.21
  @@ -25,6 +25,7 @@
   import java.util.HashMap;
   import java.util.Hashtable;
   import java.util.Iterator;
  +import java.util.List;
   import java.util.Map;
   import java.util.Vector;

  @@ -156,10 +157,20 @@

       String line = null;
       try {
  +        /**
  +         * Keep reading in the lines until we find the end of record line
  +         *
  +         * NOTE: this might chew a bit more memory than the previous implementation which
  +         * read in line blocks of 100, but allows a HUGE event spread over 100+ lines to
be decode correctly.
  +         */
           while ((line = reader.readLine()) != null) {
  -            StringBuffer buffer = new StringBuffer(line);
  -            for (int i = 0;i<100;i++) {
  -                buffer.append(reader.readLine());
  +            StringBuffer buffer = new StringBuffer(512);
  +            while(line!=null && line.indexOf(RECORD_END)==-1) {
  +                buffer.append(line).append("\n");
  +                line = reader.readLine();
  +            }
  +            if(line!=null) {
  +             buffer.append(line);
               }
               v.addAll(decodeEvents(buffer.toString()));
           }
  @@ -177,10 +188,16 @@
     }

     public Vector decodeEvents(String document) {
  +    /**
  +     * NOTE: due to the logic above, which should read in the string containing the WHOLE
event text,
  +     * there should be no need to track partial events, but I have not changed the code
just in case....
  +     *
  +     * Paul Smith 4th August 2004
  +     */
       if (document != null) {
         document = document.trim();
         if (document.equals("")) {
  -        return null;
  +        return new Vector();
         } else {
                   String newDoc=null;
                   String newPartialEvent=null;



  1.39      +7 -0      logging-log4j/src/java/org/apache/log4j/chainsaw/help/release-notes.html

  Index: release-notes.html
  ===================================================================
  RCS file: /home/cvs/logging-log4j/src/java/org/apache/log4j/chainsaw/help/release-notes.html,v
  retrieving revision 1.38
  retrieving revision 1.39
  diff -u -r1.38 -r1.39
  --- release-notes.html             31 Jul 2004 07:23:58 -0000          1.38
  +++ release-notes.html             4 Aug 2004 03:48:13 -0000           1.39
  @@ -8,6 +8,13 @@
   <h1>Release Notes</h2>

   <h1>1.99.99</h2>
  +
  +<h2>4 Aug 2004</h2>
  +<ul>
  +<li>Fixed Bug 30443 - When opening an XML file, if you hit cancel, Chainsaw opened
the file anyway</li>
  +<li>Fixed Bug 30444 - NPE when trying to open an XML file where one or more events
may be spread over a large (>100) number of lines.</li>
  +</ul>
  +
   <h2>31 July 2004</h2>
   <ul>
   <li>Corrected 'clear refine focus' bug.</li>




---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org






--

This e-mail may contain confidential and/or privileged information. If you are not the intended
recipient (or have received this e-mail in error) please notify the sender immediately and
destroy this e-mail. Any unauthorized copying, disclosure or distribution of the material
in this e-mail is strictly forbidden.

Mime
View raw message