Hello Poul-Henning,

* Poul-Henning Kamp wrote:
> In message <20081001190728.GL16837@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
WWW: http://80386.nl/

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (FreeBSD)

iEYEARECAAYFAkjj27QACgkQ52SDGA2eCwWqPgCfUKBUYLojYX tQdUm3/h71Z/gf
lzMAnjGwVodxtPVsvJ700h73MPZ7Xylk
=/2f/
-----END PGP SIGNATURE-----