Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try to repair oddball test bots timing out in test_int #119166

Merged
merged 1 commit into from
May 19, 2024

Conversation

tim-one
Copy link
Member

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

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!

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 self-assigned this May 19, 2024
@tim-one tim-one merged commit ba8af84 into python:main May 19, 2024
36 checks passed
@tim-one tim-one deleted the bbotfail branch May 19, 2024 01:55
@ned-deily
Copy link
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
Copy link
Member Author

tim-one commented May 19, 2024

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.

@ned-deily
Copy link
Member

ned-deily commented May 19, 2024

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?

@erlend-aasland
Copy link
Contributor

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

@Yhg1s
Copy link
Member

Yhg1s commented May 20, 2024

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.

@erlend-aasland
Copy link
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.

estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
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
Labels
skip issue skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants