Hello hackers,
You don't need to call stat() just to find out if a dirent is a file
or directory, most of the time. Please see attached.
From cc2f0fd4a078728a67d862e2deec0332fb8b3555 Mon Sep 17 00:00:00 2001
From: Thomas Munro
Date: Wed, 2 Sep 2020 16:15:09 +1200
Subject: [PATCH] Skip unnecessar
Thomas Munro writes:
> You don't need to call stat() just to find out if a dirent is a file
> or directory, most of the time. Please see attached.
Hm. If we do this, I can see wanting to apply the knowledge in more
places than walkdir(). Is it possible to extract out the nonstandard
bits into
On Wed, Sep 2, 2020 at 4:35 PM Tom Lane wrote:
> Thomas Munro writes:
> > You don't need to call stat() just to find out if a dirent is a file
> > or directory, most of the time. Please see attached.
>
> Hm. If we do this, I can see wanting to apply the knowledge in more
> places than walkdir(
On Thu, Sep 3, 2020 at 3:52 AM Juan José Santamaría Flecha
wrote:
> On Wed, Sep 2, 2020 at 4:35 PM Tom Lane wrote:
>> Thomas Munro writes:
>> > You don't need to call stat() just to find out if a dirent is a file
>> > or directory, most of the time. Please see attached.
>>
>> Hm. If we do this
Thomas Munro writes:
>> On Wed, Sep 2, 2020 at 4:35 PM Tom Lane wrote:
>>> Hm. If we do this, I can see wanting to apply the knowledge in more
>>> places than walkdir().
> Good idea. Here's a new version that defines a new function
> get_dirent_type() in src/common/file_utils_febe.c and uses i
On Thu, Sep 3, 2020 at 5:36 PM Tom Lane wrote:
> [request for better comments]
Ack.
> Both of these concerns would abate if we had get_dirent_type()
> just throw an error itself when stat() fails, thereby removing the
> PGFILETYPE_ERROR result code. I'm not 100% sold either way on
> that, but i
Thomas Munro writes:
> On Thu, Sep 3, 2020 at 5:36 PM Tom Lane wrote:
>> Both of these concerns would abate if we had get_dirent_type()
>> just throw an error itself when stat() fails, thereby removing the
>> PGFILETYPE_ERROR result code. I'm not 100% sold either way on
>> that, but it's somethi
On Fri, Sep 4, 2020 at 3:31 AM Tom Lane wrote:
> Thomas Munro writes:
> > Hmm. Well I had it like that in an earlier version, but then I
> > couldn't figure out the right way to write code that would work in
> > both frontend and backend code, without writing two copies in two
> > translation un
On 2020-Sep-04, Thomas Munro wrote:
> @@ -10,6 +10,7 @@ struct dirent
> {
> longd_ino;
> unsigned short d_reclen;
> + unsigned char d_type;
> unsigned short d_namlen;
> chard_name[MAX_PATH];
> };
> @@ -20,4 +21,26 @@ DIR *open
On Fri, Sep 4, 2020 at 9:37 PM Alvaro Herrera
wrote:
> On 2020-Sep-04, Thomas Munro wrote:
>
> > @@ -10,6 +10,7 @@ struct dirent
> > {
> > longd_ino;
> > unsigned short d_reclen;
> > + unsigned char d_type;
> > unsigned short d_namlen;
> > char
On 2020-Sep-04, Juan José Santamaría Flecha wrote:
> On Fri, Sep 4, 2020 at 9:37 PM Alvaro Herrera
> wrote:
>
> > On 2020-Sep-04, Thomas Munro wrote:
> > >
> > > +/* File types for 'd_type'. */
> > > +enum
> > > + {
> > > + DT_UNKNOWN = 0,
> > > +# define DT_UNKNOWN DT_UNKNOWN
> >
On Fri, Sep 4, 2020 at 10:28 PM Alvaro Herrera
wrote:
> On 2020-Sep-04, Juan José Santamaría Flecha wrote:
>
> > On Fri, Sep 4, 2020 at 9:37 PM Alvaro Herrera
> > wrote:
> >
> > > On 2020-Sep-04, Thomas Munro wrote:
>
> > > >
> > > > +/* File types for 'd_type'. */
> > > > +enum
> > > > + {
> >
On 2020-Sep-04, Juan José Santamaría Flecha wrote:
> If will fail to detect that the patch makes the optimisation available for
> WIN32:
>
> +#if defined(DT_UNKNOWN) && defined(DT_REG) && defined(DT_DIR) &&
> defined(DT_LNK)
Oh, I see. I suggest that it'd be better to change this line instead.
Alvaro Herrera writes:
> On 2020-Sep-04, Juan José Santamaría Flecha wrote:
>> If will fail to detect that the patch makes the optimisation available for
>> WIN32:
>>
>> +#if defined(DT_UNKNOWN) && defined(DT_REG) && defined(DT_DIR) &&
>> defined(DT_LNK)
> Oh, I see. I suggest that it'd be bett
Hi,
On 2020-09-02 17:51:27 +0200, Juan José Santamaría Flecha wrote:
> Win32 could also benefit from this micro-optimisation if we expanded the
> dirent port to include d_type. Please find attached a patch that does
> so
.
> }
> strcpy(d->ret.d_name, fd.cFileName);/* Both strings a
On 2020-Sep-04, Tom Lane wrote:
> I think that it's standard to test for such symbols by seeing
> if they're defined as macros ... not least because that's the *only*
> way to test their existence in C.
I guess since what we're doing is emulating standard readdir(), that
makes sense.
> Personall
On Sat, Sep 5, 2020 at 9:45 AM Andres Freund 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 he
Hi,
On 2020-09-05 11:15:07 +1200, Thomas Munro wrote:
> On Sat, Sep 5, 2020 at 9:45 AM Andres Freund 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' pa
On Sat, Sep 5, 2020 at 2:13 AM Andres Freund wrote:
>
> > 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
Hi Juan,
This is only a suggestion, if you find it appropriate.
We could use a little cut tail in get_dirent_type function.
Try to avoid add padding, when modifying or adding fields.
struct dirent
{
long d_ino;
unsigned short d_reclen;
unsigned short d_namlen;
+ unsigned char d_type;
cha
On Sun, Sep 6, 2020 at 5:23 AM Juan José Santamaría Flecha
wrote:
> On Sat, Sep 5, 2020 at 2:13 AM Andres Freund wrote:
>> > 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
Thomas Munro writes:
> Excellent. I'd like to commit these soon, unless someone has a better
> idea for how to name file_utils_febe.c.
As long as it's in src/port or src/common, isn't it implicit that
it's probably FE/BE common code?
I think it'd make more sense to insert all this stuff into fi
On Mon, Sep 7, 2020 at 10:36 AM Tom Lane wrote:
> Thomas Munro writes:
> > Excellent. I'd like to commit these soon, unless someone has a better
> > idea for how to name file_utils_febe.c.
>
> As long as it's in src/port or src/common, isn't it implicit that
> it's probably FE/BE common code?
>
On Mon, Sep 7, 2020 at 12:27 AM Thomas Munro wrote:
> On Sun, Sep 6, 2020 at 5:23 AM Juan José Santamaría Flecha
> wrote:
> > On Sat, Sep 5, 2020 at 2:13 AM Andres Freund wrote:
> >> > However, it looks like we might be missing a further opportunity
> >> > here... Doesn't Windows already give
On Mon, Sep 7, 2020 at 9:42 PM Magnus Hagander wrote:
> On Mon, Sep 7, 2020 at 12:27 AM Thomas Munro wrote:
>> I think the following is a little mysterious, but it does seem to be
>> what people do for this in other projects. It is the documented way
>> to detect mount points, and I guess IO_REP
On Mon, Sep 7, 2020 at 1:41 PM Thomas Munro wrote:
> On Mon, Sep 7, 2020 at 9:42 PM Magnus Hagander
> wrote:
> > On Mon, Sep 7, 2020 at 12:27 AM Thomas Munro
> wrote:
> >> I think the following is a little mysterious, but it does seem to be
> >> what people do for this in other projects. It is
On Tue, Sep 8, 2020 at 2:05 AM Juan José Santamaría Flecha
wrote:
> On Mon, Sep 7, 2020 at 1:41 PM Thomas Munro wrote:
>> Thanks for confirming. I ran the Windows patch through pgindent,
>> fixed a small typo, and pushed.
>
> Great, thanks. Should we include something from this discussion as com
Thomas Munro writes:
> FWIW I just spotted a couple of very suspicious looking failures for
> build farm animal "walleye", a "MinGW64 8.1.0" system, that say:
walleye's been kind of unstable since the get-go, so I wouldn't put
too much faith in reports just from it.
regar
On Tue, Sep 8, 2020 at 10:53 AM Tom Lane wrote:
> Thomas Munro writes:
> > FWIW I just spotted a couple of very suspicious looking failures for
> > build farm animal "walleye", a "MinGW64 8.1.0" system, that say:
>
> walleye's been kind of unstable since the get-go, so I wouldn't put
> too much f
On Mon, Sep 14, 2020 at 12:42 AM Thomas Munro
wrote:
> On Tue, Sep 8, 2020 at 10:53 AM Tom Lane wrote:
> > Thomas Munro writes:
> > > FWIW I just spotted a couple of very suspicious looking failures for
> > > build farm animal "walleye", a "MinGW64 8.1.0" system, that say:
> >
> > walleye's bee
30 matches
Mail list logo
