Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 1 Oct 2008 22:21:08 +0200
From:      Ed Schouten <ed@80386.nl>
To:        Poul-Henning Kamp <phk@phk.freebsd.dk>
Cc:        FreeBSD FS <freebsd-fs@freebsd.org>, Mark van Cuijk <mark@van-cuijk.nl>, Jille Timmermans <jille@quis.cx>, FreeBSD Arch <freebsd-arch@freebsd.org>
Subject:   Re: Expanding vops in vop_vectors during startup
Message-ID:  <20081001202108.GO16837@hoeg.nl>
In-Reply-To: <69186.1222890415@critter.freebsd.dk>
References:  <69186.1222890415@critter.freebsd.dk>

next in thread | previous in thread | raw e-mail | index | archive | help

--ccJhwVfaC+fHwTsl
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hello Poul-Henning,

* Poul-Henning Kamp <phk@phk.freebsd.dk> wrote:
> In message <20081001190728.GL16837@hoeg.nl>, Ed Schouten writes:
>=20
> >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.
>=20
>=20
> My resistance to this patch is not quite what you describe above:
>=20
> 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.
>=20
> 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.
>=20
> And that brings me to your "1-2%" measurement quoted in IRC and
> above.
>=20
> I have earlier ranted about the difference between benchmarking=20
> and random number generators, and you may have joined the project
> after the latest of these.
>=20
> Please search the mail-archives for that topic, and then use
> the handy ministat(1) program, to see if you have actually=20
> show any net speed benefit.
>=20
> Once and if you cross that threshold, I am going to raise my next
> objection:
>=20
> 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.

--=20
 Ed Schouten <ed@80386.nl>
 WWW: http://80386.nl/

--ccJhwVfaC+fHwTsl
Content-Type: application/pgp-signature
Content-Disposition: inline

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

iEYEARECAAYFAkjj27QACgkQ52SDGA2eCwWqPgCfUKBUYLojYXtQdUm3/h71Z/gf
lzMAnjGwVodxtPVsvJ700h73MPZ7Xylk
=/2f/
-----END PGP SIGNATURE-----

--ccJhwVfaC+fHwTsl--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20081001202108.GO16837>