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 — gh-100176: Remove outdated Tools/{io,cc,string}bench by hugovk · Pull Request #101853 · python/cpython

Conversation

@hugovk

@hugovk hugovk commented Feb 12, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Closes #32135.
Closes #100176.
Closes #100197.

See:

  • Meta issue: clean up redundant compat code #100176 (comment)
  • https://discuss.python.org/t/do-we-still-need-tools-io-cc-string-bench/22557

Replaces and:

  • closes bpo-n/a: Updated ccbench.py used to assess multithreading #32135.
  • closes gh-100176: Tools/iobench: Remove redundant compat code for Python <= 3.2 #100197.
  • Issue: Meta issue: clean up redundant compat code #100176

Sorry, something went wrong.

Uh oh!

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

erlend-aasland and arhadthedev reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@hugovk
pythongh-100176: Remove outdated Tools/{io,cc,string}bench
535c89a
@bedevere-bot bedevere-bot added the awaiting core review label Feb 12, 2023
@bedevere-bot bedevere-bot mentioned this pull request Feb 12, 2023
Meta issue: clean up redundant compat code #100176
Closed
@hugovk hugovk mentioned this pull request Feb 12, 2023
gh-100176: Tools/iobench: Remove redundant compat code for Python <= 3.2 #100197
Merged
@arhadthedev

arhadthedev commented Feb 12, 2023

Copy link
Copy Markdown
Member

BTW, it allows to close gh-32135 as outdated.

hugovk 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.

@sobolevn

sobolevn commented Feb 12, 2023

Copy link
Copy Markdown
Member

@rhettinger had some opinion on this as well.

All reactions

Sorry, something went wrong.

Uh oh!

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

@erlend-aasland

erlend-aasland commented Feb 14, 2023

Copy link
Copy Markdown
Contributor

Sad to see Håvamål go 😢👁️

cpython/Tools/iobench/iobench.py

Lines 482 to 548 in 81e3aa8

1.
Gáttir allar,
áðr gangi fram,
um skoðask skyli,
um skyggnast skyli,
því at óvíst er at vita,
hvar óvinir
sitja á fleti fyrir.
2.
Gefendr heilir!
Gestr er inn kominn,
hvar skal sitja sjá?
Mjök er bráðr,
sá er á bröndum skal
síns of freista frama.
3.
Elds er þörf,
þeims inn er kominn
ok á kné kalinn;
matar ok váða
er manni þörf,
þeim er hefr um fjall farit.
4.
Vatns er þörf,
þeim er til verðar kemr,
þerru ok þjóðlaðar,
góðs of æðis,
ef sér geta mætti,
orðs ok endrþögu.
5.
Vits er þörf,
þeim er víða ratar;
dælt er heima hvat;
at augabragði verðr,
sá er ekki kann
ok með snotrum sitr.
6.
At hyggjandi sinni
skyli-t maðr hræsinn vera,
heldr gætinn at geði;
þá er horskr ok þögull
kemr heimisgarða til,
sjaldan verðr víti vörum,
því at óbrigðra vin
fær maðr aldregi
en mannvit mikit.
7.
Inn vari gestr,
er til verðar kemr,
þunnu hljóði þegir,
eyrum hlýðir,
en augum skoðar;
svá nýsisk fróðra hverr fyrir.
8.
Hinn er sæll,
er sér of getr
lof ok líknstafi;
ódælla er við þat,
er maðr eiga skal
annars brjóstum í.

All reactions

Sorry, something went wrong.

Uh oh!

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

hugovk added 2 commits April 3, 2023 15:26
@hugovk
Merge branch 'main' into rm-tools-io-cc-stringbench
c238faf
@hugovk
Merge remote-tracking branch 'upstream/main' into rm-tools-io-cc-stri…
172e572
…ngbench
@hugovk

hugovk commented Apr 27, 2023

Copy link
Copy Markdown
Member Author

(Resolved conflict)

All reactions

Sorry, something went wrong.

Uh oh!

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

hugovk and others added 2 commits July 24, 2023 00:40
@hugovk
Merge branch 'main' into rm-tools-io-cc-stringbench
4465b93
@hugovk
Merge branch 'main' into rm-tools-io-cc-stringbench
32b75e1
@hugovk
hugovk merged commit aba37d4 into python:main Feb 17, 2024
@hugovk
hugovk deleted the rm-tools-io-cc-stringbench branch February 17, 2024 21:17
@bedevere-app bedevere-app Bot removed the awaiting core review label Feb 17, 2024
@Sophist-UK

Sophist-UK commented Feb 18, 2024

Copy link
Copy Markdown

I would have commented sooner had I known that this was about to happen. It would have been helpful if someone had commented in my PR to ccbench.py that this was being considered so that I would have been notified and might have had a chance to comment before it happened.

I believe it was clear from the extensive discussion about the shortcomings of the current GIL in faster-cpython that the ccbench script is still useful - because it clearly identifies an ongoing performance issue when multithreading with one or more CPU-heavy threads.

That said, I do understand that performance scripts do not really have a place in the cpython source code repo, but unfortunately I do not have the time to rework the updated ccbench script for the pyperformance suite, so I guess it is going to just disappear.

All reactions

Sorry, something went wrong.

Uh oh!

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

@hugovk

hugovk commented Feb 18, 2024

Copy link
Copy Markdown
Member Author

I would have commented sooner had I known that this was about to happen. It would have been helpful if someone had commented in my PR to ccbench.py that this was being considered so that I would have been notified and might have had a chance to comment before it happened.

Apologies, I'll ping next time this sort of thing comes up. (btw we wouldn't be allowed to merge your PR because it looks like the CLA isn't signed.)

That said, I do understand that performance scripts do not really have a place in the cpython source code repo, but unfortunately I do not have the time to rework the updated ccbench script for the pyperformance suite, so I guess it is going to just disappear.

Yep, in general, benchmarks are much more useful in pyperformace, because they're run often and people pay attention to their results.

The code hasn't disappeared, it's still in the 3.12 and older branches, and in Git history.

And of course, if there's consensus we can easily revert/re-add bits of this.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Sophist-UK

Sophist-UK commented Feb 18, 2024

Copy link
Copy Markdown

btw we wouldn't be allowed to merge your PR because it looks like the CLA isn't signed.

Go look at the PR history - I signed the CLA and the relevant label was added. Later the label was removed for some reason unknown to me..

All reactions

Sorry, something went wrong.

Uh oh!

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

@hugovk hugovk mentioned this pull request Feb 18, 2024
Move Benchmarks From Tools/ to the pyperformance Repo #97680
Open
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
@hugovk @woodruffw
pythongh-100176: Remove outdated Tools/{io,cc,string}bench (python#10…
e970a0e
…1853)

Co-authored-by: Hugo van Kemenade <[email protected]>
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@hugovk @diegorusso
pythongh-100176: Remove outdated Tools/{io,cc,string}bench (python#10…
85b5109
…1853)

Co-authored-by: Hugo van Kemenade <[email protected]>
LukasWoodtli pushed a commit to LukasWoodtli/cpython that referenced this pull request Jan 22, 2025
@hugovk @LukasWoodtli
pythongh-100176: Remove outdated Tools/{io,cc,string}bench (python#10…
0a23d13
…1853)

Co-authored-by: Hugo van Kemenade <[email protected]>
@StanFromIreland StanFromIreland mentioned this pull request Mar 3, 2025
Does ccbench still require 2.6 compatibility? #89801
Closed
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

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Meta issue: clean up redundant compat code

Uh oh!

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

6 participants

@hugovk @arhadthedev @sobolevn @erlend-aasland @Sophist-UK @bedevere-bot
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.