[PULL REQUEST] Vhost plugin cleanups and speedups

[Jason Stubbs]

Hi,

I've opened the following pull request:
https://github.com/perlbal/Perlbal/pull/13

Not exactly sure on the process yet, so I'll just reproduce my opening
comment below:

There's a few changes here but each change is in a separate commit.. Other
than one pure cleanup commit, the others are as follows (copied from CHANGELOG):

* Do vhost twiddling after all host header cleanups in order to standardize
with regular vhost configuration.
* Fix handling of a www.exammple.com.:80 type host header in the vhost
plugin.
* Don't check for a vhost twiddle request_uri if twiddling hasn't been
configured, so that /__using/... uris aren't needlessly appropriated.
* Return an error rather than silently continuing when a vhost maps to a
service that doesn't exist.
* Decrease overall CPU usage by 20% when using the vhost plugin.
* Allow vhost mappings to be deleted via the management port.

I'm not a perl programmer but even I know the vhost_selector optimization
isn't very perl. Turning the closure into a static function was a lot faster
(probably around 15%) but taking out the function call altogether is faster
again. Yes, I could probably use a do { } while (0); and break out of it if
need be, but I think gotos are more legible in this case... Just pre-empting
the "WTF?!" ;)

Everything else should be pretty straight forward.

--
Jason Stubbs

[Jason Stubbs]

On 4/02/12 10:36 PM, Jason Stubbs wrote:
> https://github.com/perlbal/Perlbal/pull/13

>
> * Decrease overall CPU usage by 20% when using the vhost plugin.

This turned out to be bogus. The speedup is more like 3%. I'm not sure why
the live servers had such a difference, but it must have been one of the
other changes I made where I hadn't expected any performance impact...

I've also opened a separate pull request for core changes:

https://github.com/perlbal/Perlbal/pull/14

Nothing super exciting here with only a 1% speedup, but still something.
The only point of contention would be as to whether a speedup that small
warrants a step away from "traditional" perl.

--
Jason Stubbs

[dormando]

Thanks for sharing your changes!

We're just wrapping up 1.80 after a huge set of merges, so your stuff will
be reviewed for the version after. Though we'll be trying much harder to
not have a year between cuts from now on :)

We will have to take a look at the closure refactoring to see if it's
worth it. I haven't looked at an NYTProf dump recently but we may have
some larger low hanging fruit.

but, we'll find out soon enough :) Thanks again!

[Jason Stubbs]

On 6/02/12 4:35 PM, dormando wrote:
> Thanks for sharing your changes!
>
> We're just wrapping up 1.80 after a huge set of merges, so your stuff will
> be reviewed for the version after. Though we'll be trying much harder to
> not have a year between cuts from now on :)

Yep, no problem. For the record, I'm running HEAD in production (with my
changes) running at around 160 req/s without issue.

Actually, I've had one issue unrelated to my changes that I thought I'd
fixed. Going back and looking again in preparation to send a pull request,
though, I found that I wasn't certain of the cause. I've put debugging code
in there to figure out what's going on if/when it happens again.

The issue itself was that either an fd wasn't being removed from
waiting_client_map or waiting_client_count was being incremented twice.
I can't see any code flows that could cause either, but 'queues' and 'pend
clients' from 'show service' had a 1000+ discrepancy.

> We will have to take a look at the closure refactoring to see if it's
> worth it. I haven't looked at an NYTProf dump recently but we may have
> some larger low hanging fruit.

I've just started looking at DProf, but I'll give NYTProf a try too. I've
just found though that over have the CPU time seems to be being spent in
Perl::Util::rebless(). I'm not sure that it's low hanging, but it definitely
looks like larger fruit. ;)

--
Jason Stubbs

[Jason Stubbs]

To be clear, this is with the Vhost plugin enabled.

--
Jason Stubbs

[dormando]

NYTProf is the way to go. invent a benchmark and sack it. Make sure you've
tuned perlbal's backend/client keepalive stuff most efficiently first (see
the bottom of `perldoc Perlbal::Manual::LoadBalancer`).

I've seen one other complaint about rebless, but we haven't looked into it
yet (might be some specific platforms or versions of perl).