Hi, On 2022-11-17 09:03:32 -0500, Robert Haas wrote: > On Wed, Nov 16, 2022 at 2:52 PM Andres Freund <and...@anarazel.de> wrote: > > I don't think we'd want every buffer write or whatnot go through the > > changecount mechanism, on some non-x86 platforms that could be noticable. > > But > > if we didn't stage the stats updates locally I think we could make most of > > the > > stats changes without that overhead. For updates that just increment a > > single > > counter there's simply no benefit in the changecount mechanism afaict. > > You might be right, but I'm not sure whether it's worth stressing > about. The progress reporting mechanism uses the st_changecount > mechanism, too, and as far as I know nobody's complained about that > having too much overhead. Maybe they have, though, and I've just > missed it.
I've seen it in profiles, although not as the major contributor. Most places do a reasonable amount of work between calls though. As an experiment, I added a progress report to BufferSync()'s first loop (i.e. where it checks all buffers). On a 128GB shared_buffers cluster that increases the time for a do-nothing checkpoint from ~235ms to ~280ms. If I remove the changecount stuff and use a single write + write barrier, it ends up as 250ms. Inlining brings it down a bit further, to 247ms. Obviously this is a very extreme case - we only do very little work between the progress report calls. But it does seem to show that the overhead is not entirely neglegible. I think pgstat_progress_start_command() needs the changecount stuff, as does pgstat_progress_update_multi_param(). But for anything updating a single parameter at a time it really doesn't do anything useful on a platform that doesn't tear 64bit writes (so it could be #ifdef PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY). Out of further curiosity I wanted to test the impact when the loop doesn't even do a LockBufHdr() and added an unlocked pre-check. 109ms without progress. 138ms with. 114ms with the simplified pgstat_progress_update_param(). 108ms after inlining the simplified pgstat_progress_update_param(). Greetings, Andres Freund
