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



                                                                                       
                                                
                      [EMAIL PROTECTED]                                                
                                                
                                               To:       [EMAIL PROTECTED]             
                                     
                      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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]






--

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.

Attachment: xmldecoder_patch.txt
Description: Binary data

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to