Skip to content

bpo-37146: Deactivate opcode cache only when using huntrleaks in the test suite #24643

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

Merged
merged 5 commits into from
Feb 28, 2021

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Feb 24, 2021

https://bugs.python.org/issue37146

This allows us to run the opcode cache in debug mode, as catching bugs related to the opcode cache was quite hard due to it not being activated by default in this case.

@pablogsal pablogsal requested a review from vstinner February 24, 2021 22:41
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 24, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 0372254fc3556cc2d3a7fb07f39f38b0b86ccb29 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 24, 2021
co->co_opcache_flag++;
if (co->co_opcache_flag == OPCACHE_MIN_RUNS) {
if (co->co_opcache_flag == opcache_min_runs) {
Copy link
Member

Choose a reason for hiding this comment

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

This may increase memory access in hot path. Would you run pypeformance to ensure no performance regression?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, will run this over the weekend.

Copy link
Member Author

@pablogsal pablogsal Feb 27, 2021

Choose a reason for hiding this comment

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

Seems that there is no much different over the noise level:

❯ pyperf compare_to json/* --table --min-speed 2 -G
+----------------+--------------------------------------+-----------------------------------------+
| Benchmark      | 2021-02-27_11-31-master-c71d24f55828 | 2021-02-27_19-33-bpo-37136-a9680bcf9b84 |
+================+======================================+=========================================+
| unpickle       | 17.0 us                              | 17.4 us: 1.02x slower                   |
+----------------+--------------------------------------+-----------------------------------------+
| pickle         | 13.2 us                              | 13.7 us: 1.04x slower                   |
+----------------+--------------------------------------+-----------------------------------------+
| nqueens        | 121 ms                               | 126 ms: 1.04x slower                    |
+----------------+--------------------------------------+-----------------------------------------+
| Geometric mean | (ref)                                | 1.00x slower                            |
+----------------+--------------------------------------+-----------------------------------------+

Benchmark hidden because not significant (56): xml_etree_parse, scimark_sor, unpickle_list, nbody, unpack_sequence, regex_v8, chaos, meteor_contest, telco, python_startup, genshi_text, regex_compile, python_startup_no_site, json_loads, regex_dna, logging_simple, sympy_sum, xml_etree_iterparse, scimark_sparse_mat_mult, xml_etree_process, richards, chameleon, sqlalchemy_declarative, django_template, hexiom, pathlib, json_dumps, tornado_http, sympy_str, unpickle_pure_python, pyflate, pidigits, go, regex_effbot, deltablue, sympy_expand, xml_etree_generate, float, genshi_xml, logging_silent, raytrace, fannkuch, sqlalchemy_imperative, scimark_monte_carlo, logging_format, 2to3, sympy_integrate, mako, pickle_dict, pickle_list, spectral_norm, dulwich_log, sqlite_synth, scimark_lu, pickle_pure_python, scimark_fft, crypto_pyaes

@gvanrossum
Copy link
Member

Thanks!

pablogsal and others added 3 commits February 27, 2021 19:33
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

One comment nit. This addresses my concern nicely.

pablogsal and others added 2 commits February 27, 2021 23:16
Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
@pablogsal pablogsal merged commit af5fa13 into python:master Feb 28, 2021
@pablogsal pablogsal deleted the bpo-37136 branch February 28, 2021 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants