76923 - Don't |use diagnostics;|

Status: RESOLVED FIXED

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

Description

[Zach Lipton [:zach]]

This gives us at least a 5% speedup on show_bug.cgi!

Comment 1

[Zach Lipton [:zach]]

Attachment: Junk diagnostics

Comment 2

[Zach Lipton [:zach]]

Attachment: Junk warnings

Comment 3

[Zach Lipton [:zach]]

Attachment: Headers file so that the warnings and diagnostics can be turned on easily.

Comment 4

[Matthew Tuck [:CodeMachine]]

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?

Comment 5

[Zach Lipton [:zach]]

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.

Comment 6

[Matthew Tuck [:CodeMachine]]

Not knowing much about this I'm hesitant at removing checks, but I'll move it to
2.16 for consideration.

Comment 7

[Gervase Markham [:gerv]]

CCing various people who care about performance. Eliminating this stuff looks
like an easy perf. win to me.

Gerv

Comment 8

[Dave Miller [:justdave]]

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.

Comment 9

[Matthew Tuck [:CodeMachine]]

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.

Comment 10

[Dave Miller [:justdave]]

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.

Comment 11

[Bradley Baetz (:bbaetz)]

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?

Comment 12

[Bradley Baetz (:bbaetz)]

This should be marked INVALID unless someone can back this data up.

Comment 13

[Bradley Baetz (:bbaetz)]

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/ :)

Comment 14

[Dave Miller [:justdave]]

clearing target in DUPLICATE/WORKSFORME/INVALID/WONTFIX bugs so they'll show up
as untriaged if they get reopened.

Comment 15

[Bradley Baetz (:bbaetz)]

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...

Comment 16

[Bradley Baetz (:bbaetz)]

.. and to me, for a patch

Comment 17

[Bradley Baetz (:bbaetz)]

Attachment: remove |use diagnostics;|

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.

Comment 18

[Joel Peshkin]

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.

Comment 19

[Bradley Baetz (:bbaetz)]

Checked in.