344298 - importxml.pl performance problem

Status: RESOLVED FIXED

(Bugzilla :: Bug Import/Export & Moving, defect)

Description

[Vance Baarda]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4

The importxml.pl script takes an extremely long time (half an hour or more) to process an XML file if it contains large (several MB) attachments.

I will attach a workaround for this problem.

Reproducible: Always

Steps to Reproduce:
1.Use importxml.pl to import a file that contains a 5MB attachment.
2.
3.

Actual Results:  
It takes over half an hour on a reasonably modern Pentium 4.

Expected Results:  
It should take less time. :-)

Comment 1

[Vance Baarda]

Attachment: workaround for performance problem

Comment 2

[Vance Baarda]

With this patch, importxml.pl can process a file with five 5MB attachments (total of 25MB) in 4.5 seconds on a 1400MHz Pentium III.

Without the patch, I stopped the stopwatch after 45 minutes. :-)

Comment 3

[Greg Hendricks]

Comment on attachment 228864 [details] [diff] [review]
workaround for performance problem

>+        elsif ($encoding =~ /filename/) {
>+            # read the attachment file
>+            Error("attach_path is required", undef) unless ($attach_path);
>+            my $attach_filename = $attach_path . "/" . $attach->field('data');

What happens if I am running this on Windows?

>+            Error("cannot open $attach_filename", undef) unless
>+                open(ATTACH_FH, $attach_filename);

Nit: I think this would be more readable (an more standard) as open(...) or Error()

>+            undef $/;
>+            $attachment{'data'} = <ATTACH_FH>;
>+            close ATTACH_FH;
>+            $/ = "\n";

Perl Best Practices suggests you use $variable = do { local $/; <HANDLE> };
This ensures that $/ does not get reset globally. Also, how do you know that
$/ was \n in the first place? Again, what if this was on Windows?

Comment 4

[Vance Baarda]

Attachment: take 2

Good suggestions -- thanks.

Comment 5

[Max Kanat-Alexander]

FWIW: even better is to use IO::File and use the methods for slurping in the whole file.

Also, somebody should confirm this bug if it's a confirmed problem.

Comment 6

[Greg Hendricks]

Both vrb and Myself can definitely confirm this as a legitamate problem.

Comment 7

[Greg Hendricks]

Comment on attachment 228876 [details] [diff] [review]
take 2

Works nicely.

Comment 8

[Frédéric Buclin]

tip:

Checking in importxml.pl;
/cvsroot/mozilla/webtools/bugzilla/importxml.pl,v  <--  importxml.pl
new revision: 1.62; previous revision: 1.61
done

2.22:

Checking in importxml.pl;
/cvsroot/mozilla/webtools/bugzilla/importxml.pl,v  <--  importxml.pl
new revision: 1.47.2.5; previous revision: 1.47.2.4
done