[Bug libstdc++/96484] New: Horrible performance of std::read_symlink

[Bug libstdc++/96484] New: Horrible performance of std::read_symlink

[terra at gnome dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96484

            Bug ID: 96484
           Summary: Horrible performance of std::read_symlink
           Product: gcc
           Version: 10.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: terra at gnome dot org
  Target Milestone: ---

Created attachment 49004
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49004&action=edit
Preprocessed ttt.C

This program takes ~0.25s per round in the loop for a total of minutes.  It
should take millisecons in total.

To observe the problem, "foo" needs to be a large file -- more than 500M in my
case -- that is not a symlink.  Mine is on an NFS server.


#include <filesystem>
#include <iostream>

namespace fs = std::filesystem;

int
main()
{
  for (int i = 0; i < 1000; i++) {
    std::error_code ec;
    std::cerr << i << ": " << fs::read_symlink ("foo", ec) << std::endl;
  }
}


The problem is in the implementation of read_symlink.  First an lstat call is
done (fine), but then the st_size field is used as a hint for the symlink's
size.  There is no check that the file is a symlink at this point, so in my
case a 500M buffer is allocated via an mmap system call and the kernel takes
its sweet time.

Note: a file with size 4G-1 on a 32-bit system might produce an infinite loop
as the buffer will have size 0 and get doubled from that.

Suggestion 1: if the lstat doesn't show a symlink, set the error code
right then and exit.

Suggestion 2: cap st_size at something reasonable and let the loop handle it if
it needs to be bigger.








# /usr/local/products/gcc/10.1.0/bin/g++ -v
Using built-in specs.
COLLECT_GCC=/usr/local/products/gcc/10.1.0/bin/g++
COLLECT_LTO_WRAPPER=/usr/local/products/gcc/10.1.0/lib/gcc/x86_64-suse-linux/10.1.0/lto-wrapper
Target: x86_64-suse-linux
Configured with: ../../gcc-10.1.0/configure --enable-languages=c,c++,fortran
--enable-targets=x86_64-suse-linux,i686-suse-linux
--prefix=/usr/local/products/gcc/10.1.0 --with-gnu-as
--with-as=/usr/local/products/gcc/binutils-2.32/bin/as --with-gnu-ld
--with-ld=/usr/local/products/gcc/binutils-2.32/bin/ld --enable-threads=posix
--enable-shared --enable-__cxa_atexit --enable-libstdcxx-allocator=pool
x86_64-suse-linux
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 10.1.0 (GCC) 


# uname -a
Linux monsterd09 5.0.13-1-default #1 SMP Sun May 5 15:48:04 UTC 2019 (b11e2d7)
x86_64 x86_64 x86_64 GNU/Linux

[Bug libstdc++/96484] Horrible performance of std::read_symlink

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96484

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-08-05

[Bug libstdc++/96484] Horrible performance of std::read_symlink

[terra at gnome dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96484

--- Comment #1 from M Welinder <terra at gnome dot org> ---
I should note that not only does it allocate 500M, it also zeroes it.

[Bug libstdc++/96484] Horrible performance of std::read_symlink

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96484

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:6a13a4e3f29fc4ce5eff96d74ba965c9fdc02184

commit r11-2597-g6a13a4e3f29fc4ce5eff96d74ba965c9fdc02184
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Aug 6 18:44:50 2020 +0100

    libstdc++: Fix unnecessary allocations in read_symlink [PR 96484]

    libstdc++-v3/ChangeLog:

            PR libstdc++/96484
            * src/c++17/fs_ops.cc (fs::read_symlink): Return an error
            immediately for non-symlinks.
            * src/filesystem/ops.cc (fs::read_symlink): Likewise.

[Bug libstdc++/96484] Horrible performance of std::read_symlink

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96484

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:8b7e7fcb5fff8255122a8fa94109f7ae05aeaa81

commit r10-8581-g8b7e7fcb5fff8255122a8fa94109f7ae05aeaa81
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Aug 6 18:44:50 2020 +0100

    libstdc++: Fix unnecessary allocations in read_symlink [PR 96484]

    libstdc++-v3/ChangeLog:

            PR libstdc++/96484
            * src/c++17/fs_ops.cc (fs::read_symlink): Return an error
            immediately for non-symlinks.
            * src/filesystem/ops.cc (fs::read_symlink): Likewise.

    (cherry picked from commit 6a13a4e3f29fc4ce5eff96d74ba965c9fdc02184)

[Bug libstdc++/96484] Horrible performance of std::read_symlink

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96484

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:f86f80dbccece356fa5ca7e1fd4dc581cc6a1cc4

commit r9-8787-gf86f80dbccece356fa5ca7e1fd4dc581cc6a1cc4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Aug 6 18:44:50 2020 +0100

    libstdc++: Fix unnecessary allocations in read_symlink [PR 96484]

    libstdc++-v3/ChangeLog:

            PR libstdc++/96484
            * src/c++17/fs_ops.cc (fs::read_symlink): Return an error
            immediately for non-symlinks.
            * src/filesystem/ops.cc (fs::read_symlink): Likewise.

    (cherry picked from commit 6a13a4e3f29fc4ce5eff96d74ba965c9fdc02184)

[Bug libstdc++/96484] Horrible performance of std::read_symlink

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96484

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:8b692f8b4c0e47bc8e11d9c3ab83049f68b2edbc

commit r8-10390-g8b692f8b4c0e47bc8e11d9c3ab83049f68b2edbc
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Aug 6 18:44:50 2020 +0100

    libstdc++: Fix unnecessary allocations in read_symlink [PR 96484]

    libstdc++-v3/ChangeLog:

            PR libstdc++/96484
            * src/filesystem/ops.cc (fs::read_symlink): Return an error
            immediately for non-symlinks.
            * src/filesystem/std-ops.cc (fs::read_symlink): Likewise.

    (cherry picked from commit 6a13a4e3f29fc4ce5eff96d74ba965c9fdc02184)

[Bug libstdc++/96484] Horrible performance of std::read_symlink

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96484

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |8.5
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 8.5, 9.4, 10.3 and 11.