develooper Front page | perl.perl5.porters | Postings from November 2012

Re: commit "Request is_utf8_char_slow() be inlined" has no message

Thread Previous
Karl Williamson
November 27, 2012 12:32
Re: commit "Request is_utf8_char_slow() be inlined" has no message
Message ID:
On 11/25/2012 09:22 AM, bulk88 wrote:
> Karl Williamson wrote:
>> On 11/25/2012 07:30 AM, bulk88 wrote:
>>> why? There is no commit message. How did you come to that decision?
>> I did not expand on the commit's subject line, because when it came to
>> write it, I couldn't think of anything that wasn't effectively just a
>> repeat of the subject line.
>> Perhaps I made the wrong decision.  In looking at it today, I think I
>> could have added the adjective "wrapper function" to it to make it
>> clear that there is no real meat in the to-be-inlined function.
>> I presumed that the audience would know that inlining speeds things up
>> by avoiding function call overhead, and hence in general is a "good"
>> thing.  And I presumed that they knew that the downside of inlining is
>> expansion of the memory occupied by the program.  And so, it would
>> have been better to alleviate that concern by mentioning that the
>> affected function is just a wrapper, and hence its inlining doesn't
>> appreciably change the size of the code.
> What is your cost benefit cutoff for inlining?
> Is S_is_utf8_char_slow unconditionally called? Is S_is_utf8_char_slow
> hot (profiler or benchmark or educated guess or experience)? Was it used
> in so few places (I counted 3) the overhead of the call would exceed
> inlining it?

Perl has traditionally chosen speed over space.  This function is very 
rarely called, but it consists solely of calling another function and 
adjusting its return value--something that might ordinarily be 
implemented in a macro.  Inlining it won't change very much the amount 
of memory used, but eliminates the function call overhead.  The exact 
tradeoff is platform dependent.

And there are other considerations.  This function used to be a lot 
larger than it is now.  Earlier this year, I became aware of a number of 
bugs involving inappropriate acceptance of malformed UTF-8.  Under 
deadline pressure to get 5.16 out, I refactored several functions in 
this file to fix them.  This function is supposed to reject invalid 
UTF-8, but there were malformations that it accepted.  (It did not at 
that time call other functions.)  I solved the bug by having it call 
another function that had been fixed up to work correctly (so far as is 
known).  It is complicated to do the processing correctly, and I didn't 
want to repeat the logic, so I changed this function to merely call the 
other one, and adjust its return to the boolean that this is supposed to 
return.  I saved a bunch of memory, but at the expense of a new layer of 
function call.  Also the other function does some more calculations than 
this one need do, slowing things down somewhat.  But the maintenance 
trade off seemed to me like a no-brainer, especially given the rarity 
that this function gets called.

I do try to not slow down Perl's execution unnecessarily, unless it is 
unavoidable to fix a bug.  Thus it was a no-brainer to inline this, now 
trivial, function to compensate for the increased function call 
overhead, and even if it caused extra memory to be used, it was still 
less memory than what I started with, as I had removed a bunch of code 
from it.

However, there was a bug in regen/ that prevented me from 
requesting it to be inlined.  I put fixing that on my todo list, and we 
shipped 5.16.

I finally got around to fixing regen/, and then I could do this 
item from my list.

Another consideration is that I am aware that a patch is being prepared 
that reworks a bunch of this area of the code.  I'm told that benchmarks 
indicate that there is a significant performance gain.  I wanted to get 
this code in the best shape before that patch is finalized.

Thread Previous Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About