On 28/05/19 17:10 +0100, Jonathan Wakely wrote:
PR libstdc++/90634
* src/filesystem/path.cc (path::_M_split_cmpts()): Fix check for "/".
* testsuite/27_io/filesystem/path/construct/90634.cc: New test.
* testsuite/experimental/filesystem/path/construct/90634.cc: New test.
PR libstdc++/90634
* include/bits/fs_path.h (path::path(path&&)): Only call
_M_split_cmpts() for a path with multiple components.
(path::_S_is_dir_sep()): Add missing 'static' keyword to function.
* include/experimental/bits/fs_path.h: Likewise.
* src/filesystem/path.cc (path::_M_split_cmpts()): Count number of
components and reserve space in vector. Return early when there is
only one component.
* src/filesystem/std-path.cc (path::_M_split_cmpts()): Likewise.
Tested x86_64-linux, committed to gcc-8-branch.
This is the simpler patch for trunk and gcc-9-branch, which only needs
to change std::experimental::filesystem::path, because
std::filesystem::path already avoids any unnecessary allocations.
Tested powerpc64le-linux and x86_64-w64-mingw32 Committed to trunk so
far, backport to follow.
commit cdbf389b9485c3212a59605442c392dcde76636a
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Tue May 28 16:32:57 2019 +0100
PR libstdc++/90634 reduce allocations in filesystem::path construction
PR libstdc++/90634
* include/experimental/bits/fs_path.h (path::path(path&&)): Only call
_M_split_cmpts() for a path with multiple components.
(path::_S_is_dir_sep()): Add missing 'static' keyword to function.
* src/filesystem/path.cc (path::_M_split_cmpts()): Count number of
components and reserve space in vector. Return early when there is
only one component.
* testsuite/27_io/filesystem/path/construct/90634.cc: New test.
* testsuite/experimental/filesystem/path/construct/90634.cc: New test.
diff --git a/libstdc++-v3/include/experimental/bits/fs_path.h b/libstdc++-v3/include/experimental/bits/fs_path.h
index 588f06822be..9a68a272e34 100644
--- a/libstdc++-v3/include/experimental/bits/fs_path.h
+++ b/libstdc++-v3/include/experimental/bits/fs_path.h
@@ -195,7 +195,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
path(path&& __p) noexcept
: _M_pathname(std::move(__p._M_pathname)), _M_type(__p._M_type)
{
- _M_split_cmpts();
+ if (_M_type == _Type::_Multi)
+ _M_split_cmpts();
__p.clear();
}
@@ -490,7 +491,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
return _S_convert_loc(__s.data(), __s.data() + __s.size(), __loc);
}
- bool _S_is_dir_sep(value_type __ch)
+ static bool _S_is_dir_sep(value_type __ch)
{
#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
return __ch == L'/' || __ch == preferred_separator;
diff --git a/libstdc++-v3/src/filesystem/path.cc b/libstdc++-v3/src/filesystem/path.cc
index 92def10e33e..dfc3bd53c00 100644
--- a/libstdc++-v3/src/filesystem/path.cc
+++ b/libstdc++-v3/src/filesystem/path.cc
@@ -340,6 +340,28 @@ path::_M_split_cmpts()
if (_M_pathname.empty())
return;
+ {
+ // Approximate count of components, to reserve space in _M_cmpts vector:
+ int count = 1;
+ bool saw_sep_last = _S_is_dir_sep(_M_pathname[0]);
+ bool saw_non_sep = !saw_sep_last;
+ for (value_type c : _M_pathname)
+ {
+ if (_S_is_dir_sep(c))
+ saw_sep_last = true;
+ else if (saw_sep_last)
+ {
+ ++count;
+ saw_sep_last = false;
+ saw_non_sep = true;
+ }
+ }
+ if (saw_non_sep && saw_sep_last)
+ ++count; // empty filename after trailing slash
+ if (count > 1)
+ _M_cmpts.reserve(count);
+ }
+
size_t pos = 0;
const size_t len = _M_pathname.size();
@@ -362,9 +384,13 @@ path::_M_split_cmpts()
pos = 3;
while (pos < len && !_S_is_dir_sep(_M_pathname[pos]))
++pos;
+ if (pos == len)
+ {
+ _M_type = _Type::_Root_name;
+ return;
+ }
_M_add_root_name(pos);
- if (pos < len) // also got root directory
- _M_add_root_dir(pos);
+ _M_add_root_dir(pos);
}
else
{
@@ -373,6 +399,11 @@ path::_M_split_cmpts()
_M_add_root_dir(0);
}
}
+ else if (len == 1) // got root directory only
+ {
+ _M_type = _Type::_Root_dir;
+ return;
+ }
else // got root directory
_M_add_root_dir(0);
++pos;
@@ -381,12 +412,28 @@ path::_M_split_cmpts()
else if (len > 1 && _M_pathname[1] == L':')
{
// got disk designator
+ if (len == 2)
+ {
+ _M_type = _Type::_Root_name;
+ return;
+ }
_M_add_root_name(2);
if (len > 2 && _S_is_dir_sep(_M_pathname[2]))
_M_add_root_dir(2);
pos = 2;
}
#endif
+ else
+ {
+ size_t n = 1;
+ for (; n < _M_pathname.size() && !_S_is_dir_sep(_M_pathname[n]); ++n)
+ { }
+ if (n == _M_pathname.size())
+ {
+ _M_type = _Type::_Filename;
+ return;
+ }
+ }
size_t back = pos;
while (pos < len)
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/construct/90634.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/90634.cc
new file mode 100644
index 00000000000..03ec2b1a95a
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/90634.cc
@@ -0,0 +1,78 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++17" }
+// { dg-do run { target c++17 } }
+
+#include <filesystem>
+#include <cstdlib>
+#include <testsuite_hooks.h>
+
+std::size_t bytes_allocated = 0;
+
+void* operator new(std::size_t n)
+{
+ bytes_allocated += n;
+ return std::malloc(n);
+}
+
+void operator delete(void* p) noexcept { std::free(p); }
+#if __cpp_sized_deallocation
+void operator delete(void* p, std::size_t) noexcept { std::free(p); }
+#endif
+
+void
+test01()
+{
+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+ std::wstring s0;
+ std::wstring s1 = L"/";
+ std::wstring s2 = L"///";
+ std::wstring s3 = L"file";
+ std::wstring s4 = L"C:";
+ std::wstring s5 = L"\\";
+#else
+ std::string s0;
+ std::string s1 = "/";
+ std::string s2 = "///";
+ std::string s3 = "file";
+ std::string s4 = "C:";
+ std::string s5 = "\\";
+#endif
+
+ using std::filesystem::path;
+
+ bytes_allocated = 0;
+ path p0 = std::move(s0);
+ VERIFY( bytes_allocated == 0 );
+ path p1 = std::move(s1);
+ VERIFY( bytes_allocated == 0 );
+ path p2 = std::move(s2);
+ VERIFY( bytes_allocated == 0 );
+ path p3 = std::move(s3);
+ VERIFY( bytes_allocated == 0 );
+ path p4 = std::move(s4);
+ VERIFY( bytes_allocated == 0 );
+ path p5 = std::move(s5);
+ VERIFY( bytes_allocated == 0 );
+}
+
+int
+main()
+{
+ test01();
+}
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/path/construct/90634.cc b/libstdc++-v3/testsuite/experimental/filesystem/path/construct/90634.cc
new file mode 100644
index 00000000000..6dce1df3d63
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/filesystem/path/construct/90634.cc
@@ -0,0 +1,75 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-DUSE_FILESYSTEM_TS -lstdc++fs" }
+// { dg-do run { target c++11 } }
+// { dg-require-filesystem-ts "" }
+
+#include <experimental/filesystem>
+#include <cstdlib>
+#include <testsuite_hooks.h>
+
+std::size_t bytes_allocated = 0;
+
+void* operator new(std::size_t n)
+{
+ bytes_allocated += n;
+ return std::malloc(n);
+}
+
+void operator delete(void* p) noexcept { std::free(p); }
+#if __cpp_sized_deallocation
+void operator delete(void* p, std::size_t) noexcept { std::free(p); }
+#endif
+
+void
+test01()
+{
+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+ std::wstring s0;
+ std::wstring s1 = L"/";
+ std::wstring s2 = L"file";
+ std::wstring s3 = L"C:";
+ std::wstring s4 = L"\\";
+#else
+ std::string s0;
+ std::string s1 = "/";
+ std::string s2 = "file";
+ std::string s3 = "C:";
+ std::string s4 = "\\";
+#endif
+
+ using std::experimental::filesystem::path;
+
+ bytes_allocated = 0;
+ path p0 = std::move(s0);
+ VERIFY( bytes_allocated == 0 );
+ path p1 = std::move(s1);
+ VERIFY( bytes_allocated == 0 );
+ path p2 = std::move(s2);
+ VERIFY( bytes_allocated == 0 );
+ path p3 = std::move(s3);
+ VERIFY( bytes_allocated == 0 );
+ path p4 = std::move(s4);
+ VERIFY( bytes_allocated == 0 );
+}
+
+int
+main()
+{
+ test01();
+}
