Date: Fri, 19 May 2023 07:48:43 -0700 From: Alexander Richardson <arichardson@freebsd.org> To: Hans Petter Selasky <hps@selasky.org> Cc: Colin Percival <cperciva@freebsd.org>, src-committers <src-committers@freebsd.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, dev-commits-src-main@freebsd.org Subject: Re: git: 40b287054521 - main - mi_startup: Instrument the bubblesort with TSLOG Message-ID: <CA%2BZ_v8obns91AHmY50jWRKe=Tt94fYM0SLy=vEiGzsydFJU0Lg@mail.gmail.com> In-Reply-To: <c9fd849b-5293-8cae-f08d-7e233799d3bd@selasky.org> References: <202305191349.34JDnp8J060770@gitrepo.freebsd.org> <c9fd849b-5293-8cae-f08d-7e233799d3bd@selasky.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--0000000000001c0b8c05fc0d059d Content-Type: text/plain; charset="UTF-8" On Fri, 19 May 2023, 07:07 Hans Petter Selasky, <hps@selasky.org> wrote: > On 5/19/23 15:49, Colin Percival wrote: > > The branch main has been updated by cperciva: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=40b287054521f0a92e5ae9a26e6a87d17ee85eea > > > > commit 40b287054521f0a92e5ae9a26e6a87d17ee85eea > > Author: Colin Percival <cperciva@FreeBSD.org> > > AuthorDate: 2023-05-19 13:46:42 +0000 > > Commit: Colin Percival <cperciva@FreeBSD.org> > > CommitDate: 2023-05-19 13:46:42 +0000 > > > > mi_startup: Instrument the bubblesort with TSLOG > > > > The bubblesort of SYSINITs is currently responsible for 7% of the > > kernel boot time when booting a 1 CPU / 128 MB VM under Firecracker. > > > > It needs to be replaced with a faster sort, but until that happens > > at least instrumenting it with TSLOG makes it show up in > flamecharts. > > --- > > sys/kern/init_main.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c > > index 1974c4e68ce4..e4cb501bc57b 100644 > > --- a/sys/kern/init_main.c > > +++ b/sys/kern/init_main.c > > @@ -255,6 +255,7 @@ restart: > > * Perform a bubble sort of the system initialization objects by > > * their subsystem (primary key) and order (secondary key). > > */ > > + TSENTER2("bubblesort"); > > for (sipp = sysinit; sipp < sysinit_end; sipp++) { > > for (xipp = sipp + 1; xipp < sysinit_end; xipp++) { > > if ((*sipp)->subsystem < (*xipp)->subsystem || > > @@ -266,6 +267,7 @@ restart: > > *xipp = save; > > } > > } > > + TSEXIT2("bubblesort"); > > > > last = SI_SUB_COPYRIGHT; > > #if defined(VERBOSE_SYSINIT) > > > > Hi Colin, > > If all kernel modules and the kernel could sort their SYSINIT() and > SYSUNINIT() data at compile time, then all you need to do, is to merge > two sorted lists, when loading new modules. > > Maybe this even could be part of the compiler's existing __constructor > attribute. In FreeBSD we have an example of build boot loader modules, > and statically sorting all sysinit data at compile time. See the tool I > made many years ago for this purpose: > > stand/usb/tools/sysinit.c > > What do you think? > > --HPS > A somewhat minimal improvement can be seen here: https://reviews.freebsd.org/D39916. I noticed the same slowness when booting on qemu with tracing enabled. Sorting at compile time would be ideal and in theory the priority argument in the constructor attribute should work. > --0000000000001c0b8c05fc0d059d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"auto"><div><br><br><div class=3D"gmail_quote"><div dir=3D"ltr" = class=3D"gmail_attr">On Fri, 19 May 2023, 07:07 Hans Petter Selasky, <<a= href=3D"mailto:hps@selasky.org">hps@selasky.org</a>> wrote:<br></div><b= lockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px = #ccc solid;padding-left:1ex">On 5/19/23 15:49, Colin Percival wrote:<br> > The branch main has been updated by cperciva:<br> > <br> > URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D40b287054521= f0a92e5ae9a26e6a87d17ee85eea" rel=3D"noreferrer noreferrer" target=3D"_blan= k">https://cgit.FreeBSD.org/src/commit/?id=3D40b287054521f0a92e5ae9a26e6a87= d17ee85eea</a><br> > <br> > commit 40b287054521f0a92e5ae9a26e6a87d17ee85eea<br> > Author:=C2=A0 =C2=A0 =C2=A0Colin Percival <cperciva@FreeBSD.org>= <br> > AuthorDate: 2023-05-19 13:46:42 +0000<br> > Commit:=C2=A0 =C2=A0 =C2=A0Colin Percival <cperciva@FreeBSD.org>= <br> > CommitDate: 2023-05-19 13:46:42 +0000<br> > <br> >=C2=A0 =C2=A0 =C2=A0 mi_startup: Instrument the bubblesort with TSLOG<b= r> >=C2=A0 =C2=A0 =C2=A0 <br> >=C2=A0 =C2=A0 =C2=A0 The bubblesort of SYSINITs is currently responsibl= e for 7% of the<br> >=C2=A0 =C2=A0 =C2=A0 kernel boot time when booting a 1 CPU / 128 MB VM = under Firecracker.<br> >=C2=A0 =C2=A0 =C2=A0 <br> >=C2=A0 =C2=A0 =C2=A0 It needs to be replaced with a faster sort, but un= til that happens<br> >=C2=A0 =C2=A0 =C2=A0 at least instrumenting it with TSLOG makes it show= up in flamecharts.<br> > ---<br> >=C2=A0 =C2=A0sys/kern/init_main.c | 2 ++<br> >=C2=A0 =C2=A01 file changed, 2 insertions(+)<br> > <br> > diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c<br> > index 1974c4e68ce4..e4cb501bc57b 100644<br> > --- a/sys/kern/init_main.c<br> > +++ b/sys/kern/init_main.c<br> > @@ -255,6 +255,7 @@ restart:<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 * Perform a bubble sort of the system initi= alization objects by<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 * their subsystem (primary key) and order (= secondary key).<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 */<br> > +=C2=A0 =C2=A0 =C2=A0TSENTER2("bubblesort");<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0for (sipp =3D sysinit; sipp < sysinit_end= ; sipp++) {<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0for (xipp =3D si= pp + 1; xipp < sysinit_end; xipp++) {<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0if ((*sipp)->subsystem < (*xipp)->subsystem ||<br> > @@ -266,6 +267,7 @@ restart:<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0*xipp =3D save;<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0}<br> > +=C2=A0 =C2=A0 =C2=A0TSEXIT2("bubblesort");<br> >=C2=A0 =C2=A0<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0last =3D SI_SUB_COPYRIGHT;<br> >=C2=A0 =C2=A0#if defined(VERBOSE_SYSINIT)<br> > <br> <br> Hi Colin,<br> <br> If all kernel modules and the kernel could sort their SYSINIT() and <br> SYSUNINIT() data at compile time, then all you need to do, is to merge <br> two sorted lists, when loading new modules.<br> <br> Maybe this even could be part of the compiler's existing __constructor = <br> attribute. In FreeBSD we have an example of build boot loader modules, <br> and statically sorting all sysinit data at compile time. See the tool I <br= > made many years ago for this purpose:<br> <br> stand/usb/tools/sysinit.c<br> <br> What do you think?<br> <br> --HPS<br></blockquote></div></div><div dir=3D"auto"><br></div><div dir=3D"a= uto"><br></div><div dir=3D"auto">A somewhat minimal improvement can be seen= here: <a href=3D"https://reviews.freebsd.org/D39916">https://reviews.freeb= sd.org/D39916</a>. I noticed the same slowness when booting on qemu with tr= acing enabled.</div><div dir=3D"auto"><br></div><div dir=3D"auto">Sorting a= t compile time would be ideal and in theory the priority argument in the co= nstructor attribute should work.</div><div dir=3D"auto"><div class=3D"gmail= _quote"><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border= -left:1px #ccc solid;padding-left:1ex"> </blockquote></div></div></div> --0000000000001c0b8c05fc0d059d--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2BZ_v8obns91AHmY50jWRKe=Tt94fYM0SLy=vEiGzsydFJU0Lg>