Skip to content

[Performance] Replace isinstance with attribute checks in hot code paths #535

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.

Sign up for GitHub

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

Jump to bottom
Merged
Roger-luo merged 1 commit into from
Oct 16, 2025
Merged

[Performance] Replace isinstance with attribute checks in hot code paths #535

Roger-luo merged 1 commit into from
Oct 16, 2025

Conversation

@cduck
Copy link
Contributor

@cduck cduck commented Oct 16, 2025 •
edited
Loading

Speeds up my large kernel use case by 14% from 453s to 390s (90s to 34s of isinstance checks).

@cduck cduck requested a review from Roger-luo October 16, 2025 20:38
@cduck
Copy link
Contributor Author

cduck commented Oct 16, 2025

Is this solution acceptable to you @Roger-luo?

@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2025 •
edited
Loading

PR Preview Action v1.6.2
Preview removed because the pull request was closed.
2025-10-16 20:56 UTC

@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2025

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
11329 10099 89% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
src/kirin/dialects/scf/scf2cf.py 98% 🟢
src/kirin/dialects/scf/stmts.py 80% 🟢
src/kirin/ir/exception.py 93% 🟢
src/kirin/ir/nodes/base.py 77% 🟢
src/kirin/ir/nodes/block.py 81% 🟢
src/kirin/ir/nodes/region.py 78% 🟢
src/kirin/ir/nodes/stmt.py 85% 🟢
src/kirin/ir/ssa.py 84% 🟢
src/kirin/lowering/frame.py 94% 🟢
src/kirin/rewrite/abc.py 97% 🟢
src/kirin/rewrite/walk.py 93% 🟢
TOTAL 87% 🟢

updated for commit: 4741a8d by action🐍

@codecov
Copy link

codecov bot commented Oct 16, 2025

Codecov Report

❌ Patch coverage is 93.33333% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/kirin/dialects/scf/stmts.py 50.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Roger-luo
Copy link
Collaborator

Roger-luo commented Oct 16, 2025

This looks evil... but I get why we are doing it.

@Roger-luo Roger-luo merged commit d53e5e6 into main Oct 16, 2025
12 of 13 checks passed
@Roger-luo Roger-luo deleted the cduck/isinstance-main branch October 16, 2025 20:55
Roger-luo pushed a commit that referenced this pull request Oct 16, 2025
@cduck @Roger-luo
[Performance] Replace isinstance with attribute checks in hot code pa…
9128f3c 
…ths (#535)

Speeds up my large kernel use case by 14% from 453s to 390s (90s to 34s
of isinstance checks).
@cduck
Copy link
Contributor Author

cduck commented Oct 16, 2025 •
edited
Loading

Wait @Roger-luo! You were too fast for me and merged with failing pyright type checking. Type check fix: https://github.com/QuEraComputing/kirin/tree/cduck/isinstance-main

EDIT: Still testing this fix...
EDIT: Done
https://github.com/QuEraComputing/kirin/tree/cduck/isinstance-main

Roger-luo pushed a commit that referenced this pull request Oct 16, 2025
@cduck
Type checking for #535 (#536)
f711b84
@zhenrongliew zhenrongliew mentioned this pull request Oct 22, 2025
Merged
Roger-luo pushed a commit that referenced this pull request Oct 22, 2025
@zhenrongliew
Fix Pyright complains from #535 and #536 (#539)
8046478 
Fix Pyright complains from #535 and #536 (that removed`isinstance`
causing performance issue).
Casts to correct type so pyright doesn't complain
Roger-luo pushed a commit that referenced this pull request Oct 23, 2025
@cduck @Roger-luo
Type checking for #535 (#536)
35b2419
Roger-luo pushed a commit that referenced this pull request Oct 23, 2025
@zhenrongliew @Roger-luo
Fix Pyright complains from #535 and #536 (#539)
97f77d9 
Fix Pyright complains from #535 and #536 (that removed`isinstance`
causing performance issue).
Casts to correct type so pyright doesn't complain
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Roger-luo Roger-luo Awaiting requested review from Roger-luo

Assignees

No one assigned

Labels

backport 0.17

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cduck @Roger-luo