Re: A micro-optimisation for walkdir()

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2020-09-05T00:13:32Z
Lists: pgsql-hackers
Hi,

On 2020-09-05 11:15:07 +1200, Thomas Munro wrote:
> On Sat, Sep 5, 2020 at 9:45 AM Andres Freund <andres@anarazel.de> wrote:
> > On 2020-09-02 17:51:27 +0200, Juan José Santamaría Flecha wrote:
> > > +     attrib = GetFileAttributes(d->ret.d_name);
> >
> > Is this really an optimization? The benefit of Thomas' patch is that
> > that information sometimes already is there. But here you're doing a
> > separate lookup with GetFileAttributes()?
> 
> Well as discussed already, our stat() emulation on Windows does
> multiple syscalls, so it's a slight improvement.

But the patch is patching readdir(), not just walkdir(). Not all
readdir() / ReadDir() callers necessarily do a stat() and many continue
to do a stat() after the patches. So for all of those except walkdir(),
some like RemoveOldXlogFiles() sometimes being noticable cost wise, the
patch will increase the cost on windows. No?  That's quite different
from utilizing "free" information.


> However, it looks like we might be missing a further opportunity
> here...  Doesn't Windows already give us the flags we need in the
> dwFileAttributes member of the WIN32_FIND_DATA object that the
> Find{First,Next}File() functions populate?

That'd be better...

Greetings,

Andres Freund



Commits

  1. Add d_type to our Windows dirent emulation.

  2. Skip unnecessary stat() calls in walkdir().