GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Try to repair oddball test bots timing out in test_int by tim-one · Pull Request #119166 · python/cpython

Conversation

@tim-one

@tim-one tim-one commented May 19, 2024

Copy link
Copy Markdown
Member

Various test bots (outside the ones GH normally runs) are timing out during test_int after ecd8664 (asymptotically faster str->int). Best guess is that they don't build the C _decimal module. So require that module in the most likely tests to time out then. Flying mostly blind, though!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@tim-one
Various test bots (outside the ones GH normally runs) are
7688ced
timing out during test_int after ecd8664 (asymptotically
faster str->int). Best guess is that they don't build the
C _decimal module. So require that module in the most
likely tests to time out then. Flying mostly blind, though!
@bedevere-app bedevere-app Bot added tests Tests in the Lib/test dir awaiting core review labels May 19, 2024
@tim-one tim-one added skip issue skip news and removed awaiting core review labels May 19, 2024
@tim-one tim-one self-assigned this May 19, 2024
@tim-one
tim-one merged commit ba8af84 into python:main May 19, 2024
@tim-one
tim-one deleted the bbotfail branch May 19, 2024 01:55
@ned-deily

ned-deily commented May 19, 2024

Copy link
Copy Markdown
Member

This could be, in part, due to the recent (just prior to 3.13.0b1) change (#118539) to configure option --with-system-libmpdec to now default to yes rather than no, meaning that, by default, an external ("system") version of libmpdec needs to be provided for cpython builds rather than automatically using the now-deprecated vendored libmpdec long included in the cpython source. If so, the failing buildbot configurations need to be changed to install an external libmpdec package or to specify --with-system-libmpdec=no (for 3.13). Presumably, many downstream builders of Python 3.13 will inadvertently run into this, too. @zware, @erlend-aasland, @Yhg1s

tim-one and erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tim-one

tim-one commented May 19, 2024

Copy link
Copy Markdown
Member Author

Thanks for the info, Ned! I tuned all that out while it was happening.

On the one hand, I'm happy to do what it takes so that Python doesn't need libmpdec for Python to run. decimal is a standard library, but - although CPython intended never to need to use it anymore - we still ship the pure Python workalike _pydecimal.py.

On the other ... libmpdec is a major piece of work, and is enormously faster than the Python workalike for high-precision applications. Case in point: on the failing buildbots that left behind enough clues to nail it, test_int's test_whitebox_dec_str_to_int_inner_failsafe() timed out after some 20 minutes. On my box, it takes well under 5 seconds.

It's not just C v Python speed at work. limpmdec also implements sophisticated (& very involved) numeric algorithms with far better asymptotics than _pydecimal.py, or CPython's C code, attempt. So a system that "suddenly" stops supplying the C version will sorely annoy some serious users. It's so much slower it will look to them like new bugs introduced infinite loops.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ned-deily

ned-deily commented May 19, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Yes, I think part of the issue is the name of the configure option: --with-system-libmpdec tends to imply that libmpdec is a standard system library provided by default by most operating system platforms when that's not typically the case.

@Yhg1s, I think we should consider doing something for 3.13 to make this less of a potential footgun for our downstream builders and users, if nothing else, making it less easy to overlook the fact that _decimal is accidentally no longer being built due to the lack of a libmpdec. I'm not sure what the best way to do that is. A first step might be to create or re-open an issue and mark as release blocker?

tim-one and erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented May 20, 2024

Copy link
Copy Markdown
Contributor

The --with-system-libmpdec issue should be fixed by #119196.

tim-one and ned-deily reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Yhg1s

Yhg1s commented May 20, 2024

Copy link
Copy Markdown
Member

I mentioned this to Ned yesterday in person, but yeah, I think the temporary solution is to fall back to the bundled version if the system-installed version isn't available. We may want to warn that the bundled version is going away if that's still the plan, and we can warn more aggressively going forward (even going so far as to reflect that decimal is using the bundled libmpdec in the module somewhere, so we can have tests start failing without breaking the build), but that's for later I think.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented May 20, 2024

Copy link
Copy Markdown
Contributor

@Yhg1s, yep that's what was merged in #119196; we print a warning (IIRC, autoconf outputs warnings to stderr) that a fallback is used, and that the bundled copy is deprecated and scheduled for removal.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@tim-one @estyxx
Try to repair oddball test bots timing out in test_int (python#119166)
94f028d
Various test bots (outside the ones GH normally runs) are timing out during test_int after ecd8664 (asymptotically faster str->int). Best guess is that they don't build the C _decimal module. So require that module in the most likely tests to time out then. Flying mostly blind, though!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@tim-one tim-one

Labels

skip issue skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@tim-one @ned-deily @erlend-aasland @Yhg1s
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.