1. 08 Jun, 2022 1 commit
  2. 07 Jun, 2022 1 commit
  3. 06 Jun, 2022 3 commits
  4. 03 Jun, 2022 1 commit
  5. 02 Jun, 2022 1 commit
    • Sergei Lebedev's avatar
      error_already_set::what() is now constructed lazily (#1895) · a05bc3d2
      Sergei Lebedev authored
      * error_already_set::what() is now constructed lazily
      
      Prior to this commit throwing error_already_set was expensive due to the
      eager construction of the error string (which required traversing the
      Python stack). See #1853 for more context and an alternative take on the
      issue.
      
      Note that error_already_set no longer inherits from std::runtime_error
      because the latter has no default constructor.
      
      * Do not attempt to normalize if no exception occurred
      
      This is not supported on PyPy-2.7 5.8.0.
      
      * Extract exception name via tp_name
      
      This is faster than dynamically looking up __name__ via GetAttrString.
      Note though that the runtime of the code throwing an error_already_set
      will be dominated by stack unwinding so the improvement will not be
      noticeable.
      
      Before:
      
      396 ns ± 0.913 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
      
      After:
      
      277 ns ± 0.549 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
      
      Benchmark:
      
      const std::string foo() {
          PyErr_SetString(PyExc_KeyError, "");
          const std::string &s = py::detail::error_string();
          PyErr_Clear();
          return s;
      }
      
      PYBIND11_MODULE(foo, m) {
          m.def("foo", &::foo);
      }
      
      * Reverted error_already_set to subclass std::runtime_error
      
      * Revert "Extract exception name via tp_name"
      
      The implementation of __name__ is slightly more complex than that.
      It handles the module name prefix, and heap-allocated types. We could
      port it to pybind11 later on but for now it seems like an overkill.
      
      This reverts commit f1435c7e6b068a1ed13ebd3db597ea3bb15aa398.
      
      * Cosmit following @YannickJadoul's comments
      
      Note that detail::error_string() no longer calls PyException_SetTraceback
      as it is unncessary for pretty-printing the exception.
      
      * Fixed PyPy build
      
      * Moved normalization to error_already_set ctor
      
      * Fix merge bugs
      
      * Fix more merge errors
      
      * Improve formatting
      
      * Improve error message in rare case
      
      * Revert back if statements
      
      * Fix clang-tidy
      
      * Try removing mutable
      
      * Does build_mode release fix it
      
      * Set to Debug to expose segfault
      
      * Fix remove set error string
      
      * Do not run error_string() more than once
      
      * Trying setting the tracebackk to the value
      
      * guard if m_type is null
      
      * Try to debug PGI
      
      * One last try for PGI
      
      * Does reverting this fix PyPy
      
      * Reviewer suggestions
      
      * Remove unnecessary initialization
      
      * Add noexcept move and explicit fail throw
      
      * Optimize error_string creation
      
      * Fix typo
      
      * Revert noexcept
      
      * Fix merge conflict error
      
      * Abuse assignment operator
      
      * Revert operator abuse
      
      * See if we still need debug
      
      * Remove unnecessary mutable
      
      * Report "FATAL failure building pybind11::error_already_set error_string" and terminate process.
      
      * Try specifying noexcept again
      
      * Try explicit ctor
      
      * default ctor is noexcept too
      
      * Apply reviewer suggestions, simplify code, and make helper method private
      
      * Remove unnecessary include
      
      * Clang-Tidy fix
      
      * detail::obj_class_name(), fprintf with [STDERR], [STDOUT] tags, polish comments
      
      * consistently check m_lazy_what.empty() also in production builds
      
      * Make a comment slightly less ambiguous.
      
      * Bug fix: Remove `what();` from `restore()`.
      
      It sure would need to be guarded by `if (m_type)`, otherwise `what()` fails and masks that no error was set (see update unit test). But since `error_already_set` is copyable, there is no point in releasing m_type, m_value, m_trace, therefore we can just as well avoid the runtime overhead of force-building `m_lazy_what`, it may never be used.
      
      * Replace extremely opaque (unhelpful) error message with a truthful reflection of what we know.
      
      * Fix clang-tidy error [performance-move-constructor-init].
      
      * Make expected error message less specific.
      
      * Various changes.
      
      * bug fix: error_string(PyObject **, ...)
      
      * Putting back the two eager PyErr_NormalizeException() calls.
      
      * Change error_already_set() to call pybind11_fail() if the Python error indicator not set. The net result is that a std::runtime_error is thrown instead of error_already_set, but all tests pass as is.
      
      * Remove mutable (fixes oversight in the previous commit).
      
      * Normalize the exception only locally in error_string(). Python 3.6 & 3.7 test failures expected. This is meant for benchmarking, to determine if it is worth the trouble looking into the failures.
      
      * clang-tidy: use auto
      
      * Use `gil_scoped_acquire_local` in `error_already_set` destructor. See long comment.
      
      * For Python < 3.8: `PyErr_NormalizeException` before `PyErr_WriteUnraisable`
      
      * Go back to replacing the held Python exception with then normalized exception, if & when needed. Consistently document the side-effect.
      
      * Slightly rewording comment. (There were also other failures.)
      
      * Add 1-line comment for obj_class_name()
      
      * Benchmark code, with results in this commit message.
      
                function                   #calls  test time [s]  μs / call
      master    pure_unwind                729540      1.061      14.539876
                err_set_unwind_err_clear   681476      1.040      15.260282
                err_set_error_already_set  508038      1.049      20.640525
                error_already_set_restore  555578      1.052      18.933288
                pr1895_original_foo        244113      1.050      43.018168
                                                                             PR / master
      PR #1895  pure_unwind                736981      1.054      14.295685       98.32%
                err_set_unwind_err_clear   685820      1.045      15.237399       99.85%
                err_set_error_already_set  661374      1.046      15.811879       76.61%
                error_already_set_restore  669881      1.048      15.645176       82.63%
                pr1895_original_foo        318243      1.059      33.290806       77.39%
      
      master @ commit ad146b2a
      
      Running tests in directory "/usr/local/google/home/rwgk/forked/pybind11/tests":
      ============================= test session starts ==============================
      platform linux -- Python 3.9.10, pytest-6.2.3, py-1.10.0, pluggy-0.13.1 -- /usr/bin/python3
      cachedir: .pytest_cache
      rootdir: /usr/local/google/home/rwgk/forked/pybind11/tests, configfile: pytest.ini
      collecting ... collected 5 items
      
      test_perf_error_already_set.py::test_perf[pure_unwind]
      PERF pure_unwind,729540,1.061,14.539876
      PASSED
      test_perf_error_already_set.py::test_perf[err_set_unwind_err_clear]
      PERF err_set_unwind_err_clear,681476,1.040,15.260282
      PASSED
      test_perf_error_already_set.py::test_perf[err_set_error_already_set]
      PERF err_set_error_already_set,508038,1.049,20.640525
      PASSED
      test_perf_error_already_set.py::test_perf[error_already_set_restore]
      PERF error_already_set_restore,555578,1.052,18.933288
      PASSED
      test_perf_error_already_set.py::test_perf[pr1895_original_foo]
      PERF pr1895_original_foo,244113,1.050,43.018168
      PASSED
      
      ============================== 5 passed in 12.38s ==============================
      
      pr1895 @ commit 8dff51d12e4af11aff415ee966070368fe606664
      
      Running tests in directory "/usr/local/google/home/rwgk/forked/pybind11/tests":
      ============================= test session starts ==============================
      platform linux -- Python 3.9.10, pytest-6.2.3, py-1.10.0, pluggy-0.13.1 -- /usr/bin/python3
      cachedir: .pytest_cache
      rootdir: /usr/local/google/home/rwgk/forked/pybind11/tests, configfile: pytest.ini
      collecting ... collected 5 items
      
      test_perf_error_already_set.py::test_perf[pure_unwind]
      PERF pure_unwind,736981,1.054,14.295685
      PASSED
      test_perf_error_already_set.py::test_perf[err_set_unwind_err_clear]
      PERF err_set_unwind_err_clear,685820,1.045,15.237399
      PASSED
      test_perf_error_already_set.py::test_perf[err_set_error_already_set]
      PERF err_set_error_already_set,661374,1.046,15.811879
      PASSED
      test_perf_error_already_set.py::test_perf[error_already_set_restore]
      PERF error_already_set_restore,669881,1.048,15.645176
      PASSED
      test_perf_error_already_set.py::test_perf[pr1895_original_foo]
      PERF pr1895_original_foo,318243,1.059,33.290806
      PASSED
      
      ============================== 5 passed in 12.40s ==============================
      
      clang++ -o pybind11/tests/test_perf_error_already_set.os -c -std=c++17 -fPIC -fvisibility=hidden -Os -flto -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated -Wnon-virtual-dtor -Wunused-result -isystem /usr/include/python3.9 -isystem /usr/include/eigen3 -DPYBIND11_STRICT_ASSERTS_CLASS_HOLDER_VS_TYPE_CASTER_MIX -DPYBIND11_TEST_BOOST -Ipybind11/include -I/usr/local/google/home/rwgk/forked/pybind11/include -I/usr/local/google/home/rwgk/clone/pybind11/include /usr/local/google/home/rwgk/forked/pybind11/tests/test_perf_error_already_set.cpp
      
      clang++ -o lib/pybind11_tests.so -shared -fPIC -Os -flto -shared ...
      
      Debian clang version 13.0.1-3+build2
      Target: x86_64-pc-linux-gnu
      Thread model: posix
      
      * Changing call_repetitions_target_elapsed_secs to 0.1 for regular unit testing.
      
      * Adding in `recursion_depth`
      
      * Optimized ctor
      
      * Fix silly bug in recurse_first_then_call()
      
      * Add tests that have equivalent PyErr_Fetch(), PyErr_Restore() but no try-catch.
      
      * Add call_error_string to tests. Sample only recursion_depth 0, 100.
      
      * Show lazy-what speed-up in percent.
      
      * Include real_work in benchmarks.
      
      * Replace all PyErr_SetString() with generate_python_exception_with_traceback()
      
      * Better organization of test loops.
      
      * Add test_error_already_set_copy_move
      
      * Fix bug in newly added test (discovered by clang-tidy): actually use move ctor
      
      * MSVC detects the unreachable return
      
      * change test_perf_error_already_set.py back to quick mode
      
      * Inherit from std::exception (instead of std::runtime_error, which does not make sense anymore with the lazy what)
      
      * Special handling under Windows.
      
      * print with leading newline
      
      * Removing test_perf_error_already_set (copies are under https://github.com/rwgk/rwgk_tbx/commit/7765113fbb659e1ea004c5ba24fb578244bc6cfd).
      
      * Avoid gil and scope overhead if there is nothing to release.
      
      * Restore default move ctor. "member function" instead of "function" (note that "method" is Python terminology).
      
      * Delete error_already_set copy ctor.
      
      * Make restore() non-const again to resolve clang-tidy failure (still experimenting).
      
      * Bring back error_already_set copy ctor, to see if that resolves the 4 MSVC test failures.
      
      * Add noexcept to error_already_set copy & move ctors (as suggested by @skylion007 IIUC).
      
      * Trying one-by-one noexcept copy ctor for old compilers.
      
      * Add back test covering copy ctor. Add another simple test that exercises the copy ctor.
      
      * Exclude more older compilers from using the noexcept = default ctors. (The tests in the previous commit exposed that those are broken.)
      
      * Factor out & reuse gil_scoped_acquire_local as gil_scoped_acquire_simple
      
      * Guard gil_scoped_acquire_simple by _Py_IsFinalizing() check.
      
      * what() GIL safety
      
      * clang-tidy & Python 3.6 fixes
      
      * Use `gil_scoped_acquire` in dtor, copy ctor, `what()`. Remove `_Py_IsFinalizing()` checks (they are racy: https://github.com/python/cpython/pull/28525).
      
      * Remove error_scope from copy ctor.
      
      * Add `error_scope` to `get_internals()`, to cover the situation that `get_internals()` is called from the `error_already_set` dtor while a new Python error is in flight already. Also backing out `gil_scoped_acquire_simple` change.
      
      * Add `FlakyException` tests with failure triggers in `__init__` and `__str__`
      
      THIS IS STILL A WORK IN PROGRESS. This commit is only an important resting point.
      
      This commit is a first attempt at addressing the observation that `PyErr_NormalizeException()` completely replaces the original exception if `__init__` fails. This can be very confusing even in small applications, and extremely confusing in large ones.
      
      * Tweaks to resolve Py 3.6 and PyPy CI failures.
      
      * Normalize Python exception immediately in error_already_set ctor.
      
      For background see: https://github.com/pybind/pybind11/pull/1895#issuecomment-1135304081
      
      
      
      * Fix oversights based on CI failures (copy & move ctor initialization).
      
      * Move @pytest.mark.xfail("env.PYPY") after @pytest.mark.parametrize(...)
      
      * Use @pytest.mark.skipif (xfail does not work for segfaults, of course).
      
      * Remove unused obj_class_name_or() function (it was added only under this PR).
      
      * Remove already obsolete C++ comments and code that were added only under this PR.
      
      * Slightly better (newly added) comments.
      
      * Factor out detail::error_fetch_and_normalize. Preparation for producing identical results from error_already_set::what() and detail::error_string(). Note that this is a very conservative refactoring. It would be much better to first move detail::error_string into detail/error_string.h
      
      * Copy most of error_string() code to new error_fetch_and_normalize::complete_lazy_error_string()
      
      * Remove all error_string() code from detail/type_caster_base.h. Note that this commit includes a subtle bug fix: previously error_string() restored the Python error, which will upset pybind11_fail(). This never was a problem in practice because the two PyType_Ready() calls in detail/class.h do not usually fail.
      
      * Return const std::string& instead of const char * and move error_string() to pytypes.h
      
      * Remove gil_scope_acquire from error_fetch_and_normalize, add back to error_already_set
      
      * Better handling of FlakyException __str__ failure.
      
      * Move error_fetch_and_normalize::complete_lazy_error_string() implementation from pybind11.h to pytypes.h
      
      * Add error_fetch_and_normalize::release_py_object_references() and use from error_already_set dtor.
      
      * Use shared_ptr for m_fetched_error => 1. non-racy, copy ctor that does not need the GIL; 2. enables guard against duplicate restore() calls.
      
      * Add comments.
      
      * Trivial renaming of a newly introduced member function.
      
      * Workaround for PyPy
      
      * Bug fix (oversight). Only valgrind got this one.
      
      * Use shared_ptr custom deleter for m_fetched_error in error_already_set. This enables removing the dtor, copy ctor, move ctor completely.
      
      * Further small simplification. With the GIL held, simply deleting the raw_ptr takes care of everything.
      
      * IWYU cleanup
      
      ```
      iwyu version: include-what-you-use 0.17 based on Debian clang version 13.0.1-3+build2
      ```
      
      Command used:
      
      ```
      iwyu -c -std=c++17 -DPYBIND11_TEST_BOOST -Iinclude/pybind11 -I/usr/include/python3.9 -I/usr/include/eigen3 include/pybind11/pytypes.cpp
      ```
      
      pytypes.cpp is a temporary file: `#include "pytypes.h"`
      
      The raw output is very long and noisy.
      
      I decided to use `#include <cstddef>` instead of `#include <cstdio>` for `std::size_t` (iwyu sticks to the manual choice).
      
      I ignored all iwyu suggestions that are indirectly covered by `#include <Python.h>`.
      
      I manually verified that all added includes are actually needed.
      
      Co-authored-by: default avatarAaron Gokaslan <skylion.aaron@gmail.com>
      Co-authored-by: default avatarRalf W. Grosse-Kunstleve <rwgk@google.com>
      a05bc3d2
  6. 01 Jun, 2022 1 commit
    • Aaron Gokaslan's avatar
      perf: Add object rvalue overload for accessors. Enables reference stealing (#3970) · 58802de4
      Aaron Gokaslan authored
      * Add object rvalue overload for accessors. Enables reference stealing
      
      * Fix comments
      
      * Fix more comment typos
      
      * Fix bug
      
      * reorder declarations for clarity
      
      * fix another perf bug
      
      * should be static
      
      * future proof operator overloads
      
      * Fix perfect forwarding
      
      * Add a couple of tests
      
      * Remove errant include
      
      * Improve test documentation
      
      * Add dict test
      
      * add object attr tests
      
      * Optimize STL map caster and cleanup enum
      
      * Reorder to match declarations
      
      * adjust increfs
      
      * Remove comment
      
      * revert value change
      
      * add missing move
      58802de4
  7. 31 May, 2022 3 commits
  8. 28 May, 2022 2 commits
  9. 27 May, 2022 1 commit
  10. 26 May, 2022 2 commits
  11. 25 May, 2022 1 commit
  12. 24 May, 2022 1 commit
  13. 23 May, 2022 1 commit
  14. 20 May, 2022 1 commit
  15. 19 May, 2022 1 commit
  16. 17 May, 2022 2 commits
  17. 16 May, 2022 4 commits
  18. 09 May, 2022 1 commit
  19. 06 May, 2022 1 commit
  20. 05 May, 2022 2 commits
  21. 04 May, 2022 2 commits
  22. 02 May, 2022 2 commits
  23. 26 Apr, 2022 1 commit
  24. 24 Apr, 2022 4 commits