Skip to content

Commit fa892ac

Browse files
author
Release Manager
committed
gh-41178: Speedup Integer + int Before: ``` sage: a = ZZ(5) sage: %timeit a + 1r 295 ns ± 5.14 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each) sage: %timeit a + ZZ.one() 114 ns ± 1.1 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each) ``` After: ``` sage: a = ZZ(5) sage: %timeit a + 1r 41.3 ns ± 0.354 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each) sage: %timeit 1r + a 51.6 ns ± 0.692 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each) sage: %timeit a + ZZ.one() 120 ns ± 1.22 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each) ``` as far as I can tell, the increase in the last one is unrelated. note that the overhead comes from the cached function call ``` sage: a = ZZ(5) sage: b = ZZ.one() sage: %timeit a + b 49.1 ns ± 2.81 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each) ``` ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] The title is concise and informative. - [ ] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - #12345: short description why this is a dependency --> <!-- - #34567: ... --> URL: #41178 Reported by: user202729 Reviewer(s):
2 parents 48b3efd + c2ee523 commit fa892ac

File tree

2 files changed

+40
-6
lines changed

2 files changed

+40
-6
lines changed

src/sage/libs/mpmath/ext_impl.pyx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ cdef inline void mpz_add_si(mpz_t a, mpz_t b, long x) noexcept:
5454
if x >= 0:
5555
mpz_add_ui(a, b, x)
5656
else:
57-
# careful: overflow when negating INT_MIN
58-
mpz_sub_ui(a, b, <unsigned long>(-x))
57+
# careful: overflow when negating LONG_MIN
58+
mpz_sub_ui(a, b, -<unsigned long>x)
5959

6060
cdef inline mpzi(mpz_t n):
6161
return mpz_get_pyintlong(n)

src/sage/rings/integer.pyx

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ new_gen_from_integer = None
205205

206206

207207
cdef extern from *:
208+
int likely(int) nogil
208209
int unlikely(int) nogil # Defined by Cython
209210

210211
cdef extern from "Python.h":
@@ -433,13 +434,43 @@ cdef inline Integer move_integer_from_mpz(mpz_t x):
433434
``x`` will not be cleared;
434435
435436
- if ``sig_on()`` does not throw, :func:`move_integer_from_mpz` will call ``mpz_clear(x)``.
437+
438+
Note that this is in fact slightly slower than ::
439+
440+
cdef Integer x = <Integer>PY_NEW(Integer)
441+
mpz_SOMETHING_MUTATE_X(x.value, ...)
442+
return x
443+
444+
because with ``move_integer_from_mpz``, one need to allocate a new ``mpz_t``, even if
445+
the ``x`` returned by ``PY_NEW`` already have an allocated buffer (see :func:`fast_tp_new`).
446+
Only use this when interruptibility is required.
436447
"""
437448
cdef Integer y = <Integer>PY_NEW(Integer)
438449
mpz_swap(y.value, x)
439450
mpz_clear(x)
440451
return y
441452

442453

454+
cdef Integer integer_add_python_int(Integer left, right):
455+
"""
456+
Internal helper method. Return ``left + right``, where ``right`` must be an ``int``.
457+
"""
458+
cdef Integer x
459+
cdef int overflow
460+
cdef long tmp
461+
x = <Integer>PY_NEW(Integer)
462+
tmp = PyLong_AsLongAndOverflow(right, &overflow)
463+
if overflow == 0:
464+
if tmp >= 0:
465+
mpz_add_ui(x.value, left.value, tmp)
466+
else:
467+
mpz_sub_ui(x.value, left.value, -<unsigned long>tmp)
468+
else:
469+
mpz_set_pylong(x.value, right)
470+
mpz_add(x.value, left.value, x.value)
471+
return x
472+
473+
443474
cdef class Integer(sage.structure.element.EuclideanDomainElement):
444475
r"""
445476
The :class:`Integer` class represents arbitrary precision
@@ -1825,16 +1856,19 @@ cdef class Integer(sage.structure.element.EuclideanDomainElement):
18251856
sage: 1 + (-2/3)
18261857
1/3
18271858
"""
1859+
# because of c_api_binop_methods, either left or right is Integer
18281860
cdef Integer x
18291861
cdef Rational y
1830-
if type(left) is type(right):
1831-
x = <Integer>PY_NEW(Integer)
1832-
mpz_add(x.value, (<Integer>left).value, (<Integer>right).value)
1833-
return x
1862+
if likely(type(left) is type(right)):
1863+
return (<Integer> left)._add_(right)
18341864
elif type(right) is Rational:
18351865
y = <Rational>PY_NEW(Rational)
18361866
mpq_add_z(y.value, (<Rational>right).value, (<Integer>left).value)
18371867
return y
1868+
elif type(right) is int:
1869+
return integer_add_python_int(<Integer>left, right)
1870+
elif type(left) is int:
1871+
return integer_add_python_int(<Integer>right, left)
18381872

18391873
return coercion_model.bin_op(left, right, operator.add)
18401874

0 commit comments

Comments
 (0)