Expanding vops in vop_vectors during startup

[Ed Schouten]

Hello Poul-Henning,

* Poul-Henning Kamp <phk at phk.freebsd.dk> wrote:
> In message <20081001190728.GL16837 at hoeg.nl>, Ed Schouten writes:
> 
> >The reason I'm sending this message, is because based on discussions I
> >had with several people on IRC we've basically got two different
> >opinions on this patch:
> >
> >- One group of people liked the idea of the patch. Some people even said
> >  the patch looks good enough to be committed.
> >
> >- Another group of people also liked the idea, but thought it would make
> >  no sense to commit it, because it's not like it's a bottleneck right
> >  now. It should only be committed if an increase in performance is
> >  notable.
> >
> >I did some tests with the patch set, by running tens of millions of
> >fstat(), fchown(), etc. calls to see how performance was affected. It
> >turns out on a kernel without any debugging options enabled, the
> >performance gain is only 1-2%, which sounds pretty valid to me.
> 
> 
> My resistance to this patch is not quite what you describe above:
> 
> By factoring the vop vectors out, you remove the ability to let
> default vectors pick up new functionality later.

Could you give me an example of such functionality? You mean extending a
vop_vector? That shouldn't be a problem, right? If such functionality
really seems to be in conflict with this modification, it's not like
we're carving things in stone here.

> I will admit that I have no knowledge of this level of generality,
> dating back from Heidemans Phd. dissertation, being used for anything
> sensible.
> 
> Furthermore, if I am not mistaken, your patch increases the kernel
> size.

Even though I admit I don't have that many file system types compiled
into my kernel, binary size is 2203 bytes smaller with my patch applied.
If you have a whole bunch of filesystems compiled into your kernel,
these numbers might be a little different. We now need an extra SYSINIT
per struct vop_vector.

> Absent a plausible performance improvement, I don't see any point
> of your change.
> 
> And that brings me to your "1-2%" measurement quoted in IRC and
> above.
> 
> I have earlier ranted about the difference between benchmarking 
> and random number generators, and you may have joined the project
> after the latest of these.
> 
> Please search the mail-archives for that topic, and then use
> the handy ministat(1) program, to see if you have actually 
> show any net speed benefit.
> 
> Once and if you cross that threshold, I am going to raise my next
> objection:
> 
> Benchmarking "tens of millions of fstat(), fchown(), etc. calls"
> and showing a 1-2% difference is patently bogus, and certainly
> no reason for the change you propose.

ministat(1) also mentions a 2% improvement with 95.0% confidence. Quite
a nifty tool. Thanks for pointing me to it.

About the benchmarks: the reason why I decided to test these things, was
because I didn't want to measure disk I/O performance. I just wanted to
see how performance was different with respect to VOP_*() calls. This
means most of the cases I tested scenario's when data would already be
available from cache or on pseudo-filesystems, where real disk I/O would
not occur.

But as I said: I am not going to commit it. End of discussion.

-- 
 Ed Schouten <ed at 80386.nl>
 WWW: http://80386.nl/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-arch/attachments/20081001/9b5ea7bd/attachment.pgp