Page MenuHomeFreeBSD

Add setproctitle_fast() for frequent callers.
ClosedPublic

Authored by munro_ip9.org on Jul 3 2018, 12:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 19 2024, 6:41 AM
Unknown Object (File)
Jan 10 2024, 10:28 PM
Unknown Object (File)
Dec 20 2023, 12:13 AM
Unknown Object (File)
Nov 17 2023, 7:11 AM
Unknown Object (File)
Oct 30 2023, 8:01 AM
Unknown Object (File)
Oct 19 2023, 8:24 PM
Unknown Object (File)
Oct 19 2023, 5:59 PM
Unknown Object (File)
Oct 18 2023, 1:48 PM
Subscribers

Details

Summary

"Some applications, notably PostgreSQL, want to call setproctitle()
very often. It's slow. Provide an alternative cheap way of updating
process titles without making any syscalls, instead requiring other
processes (top, ps etc) to do a bit more work to retrieve the data.
This uses a pre-existing code path inherited from ancient BSD, which
always did it that way."

As discussed in https://lists.freebsd.org/pipermail/freebsd-hackers/2018-July/052973.html .

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

You need to add the new symbol to the gen/Symbol.map, otherwise it is not possible to link to it.

Please use -U99999 argument to the diff. eg. svn diff -x -U999999 when uploading the diff, to provide full context.

lib/libc/gen/setproctitle.3
45 ↗(On Diff #44801)

... slower, by not updating the kernel cache of the program arguments.

lib/libc/gen/setproctitle.c
58 ↗(On Diff #44801)

I do not think that inline is needed there, compilers are very eager to inline on their own.

76 ↗(On Diff #44801)

return (NULL);

83 ↗(On Diff #44801)

Same. I do not repeat this note any more.

177 ↗(On Diff #44801)

if (buf && !fast_update) {

mjg added inline comments.
lib/libc/gen/setproctitle.c
183 ↗(On Diff #44801)

It's not a requirement for this patch, but it would be nice to sort out kernel-side while doing work in the area. In particular, there is no reason to grab the pid -- passing 0 should be treated as "this process".

Updates from kib's code review.

munro_ip9.org added inline comments.
lib/libc/gen/setproctitle.c
58 ↗(On Diff #44801)

clang 6.0 -O2 doesn't seem to inline it whether I say that or not. I could use attribute((always_inline)) but that seems unorthodox. I just removed it.

munro_ip9.org added inline comments.
lib/libc/gen/setproctitle.c
183 ↗(On Diff #44801)

Good point. I will look into writing a separate patch for that.

I will fix style issues myself, but I think it is useful for you to see them listed explicitly.
I am going to commit this now.

lib/libc/gen/setproctitle.3
41 ↗(On Diff #44834)

New sentence requires new line.

lib/libc/gen/setproctitle.c
158 ↗(On Diff #44834)

return ();

This revision is now accepted and ready to land.Jul 4 2018, 12:52 PM
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state Needs Review.Jul 4 2018, 1:28 PM
This revision was automatically updated to reflect the committed changes.

Thanks kib! For the record, here's a patch for PostgreSQL that I plan to push when this makes it to a RELEASE:

https://www.postgresql.org/message-id/CAEepm%3D1wKMTi81uodJ%3D1KbJAz5WedOg%3Dcr8ewEXrUFeaxWEgww%40mail.gmail.com

Thanks kib! For the record, here's a patch for PostgreSQL that I plan to push when this makes it to a RELEASE:

https://www.postgresql.org/message-id/CAEepm%3D1wKMTi81uodJ%3D1KbJAz5WedOg%3Dcr8ewEXrUFeaxWEgww%40mail.gmail.com

Why waiting for a release ? There is a significant population of users utilizing stable, and even HEAD is sometimes used not only by developers. Early you push this into the wild world, more feedback you get.

In D16111#342204, @kib wrote:

Thanks kib! For the record, here's a patch for PostgreSQL that I plan to push when this makes it to a RELEASE:

https://www.postgresql.org/message-id/CAEepm%3D1wKMTi81uodJ%3D1KbJAz5WedOg%3Dcr8ewEXrUFeaxWEgww%40mail.gmail.com

Why waiting for a release ? There is a significant population of users utilizing stable, and even HEAD is sometimes used not only by developers. Early you push this into the wild world, more feedback you get.

https://reviews.freebsd.org/D16234 to add the patches to the FreeBSD ports for stable PostgreSQL releases. I'll commit the change to the PostgreSQL development branch soon, to affect PostgreSQL 12.

I'll commit the change to the PostgreSQL development branch soon, to affect PostgreSQL 12.

Done.

This revision is now accepted and ready to land.Aug 30 2018, 10:12 PM