FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-130167: Improve speed of `_pydecimal._all_zeros` and `_pydecimal._exact_half` by replacing `re` by Marius-Juston · Pull Request #132065 · python/cpython · GitHub

/ cpython Public

gh-130167: Improve speed of _pydecimal._all_zeros and _pydecimal._exact_half by replacing re - #132065

Closed
Marius-Juston wants to merge 10 commits into
python:mainfrom
Marius-Juston:_pydecimal
ConversationCommits (10)ChecksFiles changed
Closed

gh-130167: Improve speed of _pydecimal._all_zeros and _pydecimal._exact_half by replacing re#132065
Marius-Juston wants to merge 10 commits into
python:mainfrom
Marius-Juston:_pydecimal

Conversation

Marius-Juston commented Apr 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Benchmark for all zeros

Benchmark regex_decimal non_regex_ends_fast_decimal
exact 230 ns 83.5 ns: 2.76x faster
No zeros 1 199 ns 83.9 ns: 2.38x faster
No zeros 10 207 ns 84.1 ns: 2.46x faster
No zeros 100 206 ns 86.4 ns: 2.38x faster
No zeros 1000 201 ns 94.1 ns: 2.13x faster
Mixed 1 204 ns 169 ns: 1.21x faster
Mixed 10 200 ns 170 ns: 1.18x faster
Mixed 100 208 ns 175 ns: 1.19x faster
Mixed 1000 198 ns 231 ns: 1.17x slower
Zero 1 231 ns 150 ns: 1.54x faster
Zero 10 237 ns 180 ns: 1.32x faster
Zero 100 287 ns 185 ns: 1.55x faster
Zero 1000 783 ns 245 ns: 3.20x faster
Geometric mean (ref) 1.72x faster

Benchmark for exact half

Benchmark half_regex half_non_regex_correct
exact 176 ns 97.1 ns: 1.81x faster
No half 1 178 ns 124 ns: 1.44x faster
No half 10 177 ns 125 ns: 1.42x faster
No half 100 177 ns 125 ns: 1.42x faster
No half 1000 180 ns 126 ns: 1.42x faster
Mixed 1 177 ns 126 ns: 1.41x faster
Mixed 10 177 ns 124 ns: 1.42x faster
Mixed 100 178 ns 125 ns: 1.42x faster
Mixed 1000 177 ns 127 ns: 1.39x faster
Half 1 232 ns 230 ns: 1.01x faster
Half 10 235 ns 269 ns: 1.14x slower
Half 100 288 ns 268 ns: 1.08x faster
Half 1000 780 ns 338 ns: 2.31x faster
Geometric mean (ref) 1.38x faster

The tests benchmarks

import pyperf

def setup_tests_zero():
    names = [
        ('exact', Decimal('3.5')._int)
    ]

    for i in [1, 10, 100, 1000]:
        names.append((f'No zeros {i}', Decimal('3.1' + '12' * i)._int))

    for i in [1, 10, 100, 1000]:
        names.append((f'Mixed {i}', Decimal('3.12' + '0' * i)._int))
        
    for i in [1, 10, 100, 1000]:
        names.append((f'Zero {i}', Decimal('3.1' + '0' * i)._int))
        
    return names

def setup_tests_half():
    names = [
        ('exact', Decimal('3.5')._int)
    ]

    for i in [1, 10, 100, 1000]:
        names.append((f'No half {i}', Decimal('3.1' + '12' * i)._int))

    for i in [1, 10, 100, 1000]:
        names.append((f'Mixed {i}', Decimal('3.5' + '0' * i)._int))
        
    for i in [1, 10, 100, 1000]:
        names.append((f'Half {i}', Decimal('3.15' + '0' * i)._int))
        
    return names

method = ''
benchmark_method = 'half'

benchmark = setup_tests_half if benchmark_method == 'half' else set 

func = _round_down if method == 'regex' else _is_all_zeros

runner = pyperf.Runner()

for n, v in benchmark():
    # print(func(v))
    runner.bench_func(n, func, v)

bedevere-app Bot mentioned this pull request Apr 4, 2025
Open
eendebakpt reviewed Apr 4, 2025
View reviewed changes
Comment thread Lib/_pydecimal.py Outdated
eendebakpt reviewed Apr 4, 2025
View reviewed changes
Comment thread Lib/_pydecimal.py Outdated

eendebakpt commented Apr 4, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

I have some doubts this is worthwhile. The _all_zeros is indeed faster, but is that a bottleneck in actual computations? I would like to see at least a benchmark with a public method from the decimal module and a benchmark with huge numbers.

eendebakpt reviewed Apr 4, 2025
View reviewed changes
Comment thread Lib/_pydecimal.py Outdated
_all_zeros = re.compile('0*$').match
_exact_half = re.compile('50*$').match
# Checks for regex 0*$
def _all_zeros(d_int,prec=0):

eendebakpt Apr 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Do we need the prec argument?

Marius-Juston Apr 4, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

yes we need it, since when it gets called in:

def _round_down(self, prec):
    """Also known as round-towards-0, truncate."""
    if _all_zeros(self._int, prec):
        return 0
    else:
        return -1

and

def _round_half_up(self, prec):
    """Rounds 5 up (away from 0)"""
    if self._int[prec] in '56789':
        return 1
    elif _all_zeros(self._int, prec):
        return 0
    else:
        return -1

they both need the argument

Marius-Juston and others added 3 commits April 4, 2025 07:03
Update Lib/_pydecimal.py
b6a8014 
Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>
Update Lib/_pydecimal.py
027d93d 
Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>
simplified and sped up _all_zeros compute
34486dd

Marius-Juston commented Apr 4, 2025

Copy link
Copy Markdown
Contributor Author
Benchmark regex_decimal non_regex_ends_fast_decimal non_regex_while
exact 230 ns 83.5 ns: 2.76x faster 119 ns: 1.93x faster
No zeros 1 199 ns 83.9 ns: 2.38x faster 119 ns: 1.67x faster
No zeros 10 207 ns 84.1 ns: 2.46x faster 119 ns: 1.74x faster
No zeros 100 206 ns 86.4 ns: 2.38x faster 119 ns: 1.74x faster
No zeros 1000 201 ns 94.1 ns: 2.13x faster 127 ns: 1.59x faster
Mixed 1 204 ns 169 ns: 1.21x faster 119 ns: 1.72x faster
Mixed 10 200 ns 170 ns: 1.18x faster 119 ns: 1.68x faster
Mixed 100 208 ns 175 ns: 1.19x faster 120 ns: 1.74x faster
Mixed 1000 198 ns 231 ns: 1.17x slower 126 ns: 1.57x faster
Zero 1 231 ns 150 ns: 1.54x faster 120 ns: 1.92x faster
Zero 10 237 ns 180 ns: 1.32x faster 120 ns: 1.98x faster
Zero 100 287 ns 185 ns: 1.55x faster 120 ns: 2.40x faster
Zero 1000 783 ns 245 ns: 3.20x faster 126 ns: 6.20x faster
Geometric mean (ref) 1.72x faster 1.97x faster

with the new while loop ( should have done this method first lol )

Marius-Juston commented Apr 4, 2025

Copy link
Copy Markdown
Contributor Author
Benchmark half_regex half_non_regex_correct half_non_regex_while
exact 176 ns 97.1 ns: 1.81x faster 125 ns: 1.41x faster
No half 1 178 ns 124 ns: 1.44x faster 127 ns: 1.40x faster
No half 10 177 ns 125 ns: 1.42x faster 126 ns: 1.40x faster
No half 100 177 ns 125 ns: 1.42x faster 126 ns: 1.41x faster
No half 1000 180 ns 126 ns: 1.42x faster 128 ns: 1.40x faster
Mixed 1 232 ns 230 ns: 1.01x faster 126 ns: 1.84x faster
Mixed 10 235 ns 269 ns: 1.14x slower 126 ns: 1.87x faster
Mixed 100 288 ns 268 ns: 1.08x faster 126 ns: 2.28x faster
Mixed 1000 780 ns 338 ns: 2.31x faster 128 ns: 6.10x faster
Half 1 177 ns 126 ns: 1.41x faster 126 ns: 1.41x faster
Half 10 177 ns 124 ns: 1.42x faster 126 ns: 1.40x faster
Half 100 178 ns 125 ns: 1.42x faster 126 ns: 1.41x faster
Half 1000 177 ns 127 ns: 1.39x faster 128 ns: 1.38x faster
Geometric mean (ref) 1.38x faster 1.70x faster

With the new while loop method for the _exact_half as well

Marius-Juston commented Apr 4, 2025

Copy link
Copy Markdown
Contributor Author

I have some doubts this is worthwhile. The _all_zeros is indeed faster, but is that a bottleneck in actual computations? I would like to see at least a benchmark with a public method from the decimal module and a benchmark with huge numbers.

This is probably not a bottleneck in actual computation ( it's a nice and easy optimization for sure ) and i was planning to use public methods later for benchmarking ( separating them this way though made it simpler to optimize individually ). And yeah I was planning to do larger numbers as well to test this out with repeating patterns as well to see. ( though since the prec sets the initial location of the match or the while loop it does not actually really matter how large the number is but really what the precision is )

Though these addition benchmarks will take me some time to do since I will be very busy, so if you wanted to test it yourself feel free!

picnixz commented Apr 4, 2025

Copy link
Copy Markdown
Member

I don't think this change needs to be done. _pydecimal is only used as a fallback when the decimal C extension is not available. In addition, this is not really better from a readability PoV, so I would prefer keeping this as is.

skirpichev commented Apr 4, 2025

Copy link
Copy Markdown
Member

This is probably not a bottleneck in actual computation ( it's a nice and easy optimization for sure )

If so, hardly it's a "nice optimization". E.g. the re dependency is not removed.

BTW, what's you use case for the pure-Python decimal module? In most cases, people will just use C-coded extension.

Marius-Juston commented Apr 4, 2025

Copy link
Copy Markdown
Contributor Author

Fair enough, I don't actually have a use case for the _pydecimal I was just looking through files and functions to see if there was anything that interested me

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

eendebakpt eendebakpt left review comments

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL