Closed Bug 76923 Opened 20 years ago Closed 18 years ago

Don't |use diagnostics;|

Categories

(Bugzilla :: Creating/Changing Bugs, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: zach, Assigned: bbaetz)

References

Details

(Keywords: perf)

Attachments

(1 file, 3 obsolete files)

This gives us at least a 5% speedup on show_bug.cgi!
Blocks: bz-perf
Attached patch Junk diagnostics (obsolete) — Splinter Review
Attached patch Junk warnings (obsolete) — Splinter Review
Umm AFAICS this is all stuff that would be compile-time in a non-interpreted 
language.  Wouldn't investigating that Apache Perl-compilation module thingo be 
better?
Modperl is all good and fine (note that the patch doesn't quite work yet...), 
but it requires lots of work to set it up on the installation end. I would rather 
try to eerk out all the proformence I can from the actual perl scripts (where 
we spend 10% of the time in show_bug.cgi doing warnings and 
diagnostics) than going through gross hacks that most people won't take 
the time and effort to use. If you are interested in working on this, you are 
welcome to file a bug assigned to you that blocks the meta bug 75488.
Not knowing much about this I'm hesitant at removing checks, but I'll move it to
2.16 for consideration.
Target Milestone: --- → Bugzilla 2.16
Priority: -- → P3
Keywords: patch, review
Component: Bugzilla → Creating/Changing Bugs
Product: Webtools → Bugzilla
Version: other → unspecified
CCing various people who care about performance. Eliminating this stuff looks
like an easy perf. win to me.

Gerv
Keywords: perf
I'm iffy.  If the warnings aren't generated, we don't get bug reports that we
need to be getting.  5% isn't THAT big of a speedup, and there's other things we
can pursue for speed.
I notice a distinct lack of statistics here.  What is this trying to improve?

Is it 5% of 5 seconds or of 0.2 seconds?

What is slow that you're trying to improve?

Is it 5% of script execution time, or fetch time.  Network and browser layout 
will add to the time taken and we can't save that here.

I would call this a bigger hack than mod_perl.
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
I find it hard to believe that use strict/diagnositics is 5% of anything apart
from a totally script which has nothing else in it. Can we get some numbers?
This should be marked INVALID unless someone can back this data up.
INVALID. If you have a specific non-empty test case where warnings take a
noticable ammount of time, file a bug at http://bugs.perl.com/ :)
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → INVALID
clearing target in DUPLICATE/WORKSFORME/INVALID/WONTFIX bugs so they'll show up
as untriaged if they get reopened.
Target Milestone: Bugzilla 2.18 → ---
It turns out that this is, in fact valid. When I testedl I was testing run time
perf. |diagnostics| is not measurable for that, but the problem is that it
parses perldiag.pod at _compile_ time, so to measure the change you need to
remove it from everywhere.

This takes 0.25 seconds, or roughly 1/4 of our startup time.

Furthermore, its not necessary, and it clutters the logs.

reopening...
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
.. and to me, for a patch
Assignee: zach → bbaetz
Status: REOPENED → NEW
Target Milestone: --- → Bugzilla 2.18
Theres no point in doing the more complicated scheme suggested earlier;
warnings aren't an issue, and |perldoc perldiag| will deal with any obscure
error messages.
Attachment #31632 - Attachment is obsolete: true
Attachment #31633 - Attachment is obsolete: true
Attachment #31634 - Attachment is obsolete: true
Comment on attachment 96648 [details] [diff] [review]
remove |use diagnostics;|

2xr=joel
As new modules are added, it becomes even more important to add them to the
test suite.  We now rely on -w.
Attachment #96648 - Flags: review+
Checked in.
Status: NEW → RESOLVED
Closed: 19 years ago18 years ago
Resolution: --- → FIXED
Summary: Don't do diagnostics and warnings unless wanted → Don't |use diagnostics;|
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.