8000 Swapped out static tag approach with `ClassValue` cache by djspiewak · Pull Request #4266 · typelevel/cats-effect · GitHub
[go: up one dir, main page]
Skip to content

Swapped out static tag approach with ClassValue cache #4266

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
Open
djspiewak wants to merge 3 commits into
base: series/3.6.x
Choose a base branch
from
Open

Swapped out static tag approach with ClassValue cache #4266

djspiewak wants to merge 3 commits into from

Conversation

@djspiewak
Copy link
Member
@djspiewak djspiewak commented Feb 5, 2025 •
edited
Loading

All credit goes to @xuwei-k here. I even cribbed his code more or less directly. I haven't yet run the full set of benchmarks so hold off on merging.

Fixes #4260

@djspiewak djspiewak changed the title Swapped out static tag approach with ClassValue cache (ht @xuwei-k) Swapped out static tag approach with ClassValue cache Feb 5, 2025
@armanbilge armanbilge linked an issue Feb 5, 2025 that may be closed by this pull request
use java.lang.ClassValue instead of IO.tag #4260
Open
@djspiewak
Copy link
Member Author
djspiewak commented Feb 6, 2025

Ah, unfortunate discovery: ClassValue doesn't exist on Scala Native!

@armanbilge armanbilge mentioned this pull request Feb 7, 2025
Open
@armanbilge
Copy link
Member
armanbilge commented Feb 7, 2025

Darn, unless we do some messy source-splitting, that means we won't get this in until we complete the 0.5 upgrade ... anyway I opened an issue.

@djspiewak
Copy link
Member Author
djspiewak commented Feb 7, 2025

Yeah I briefly thought about doing some sort of weird split thing but it would be bonkers. So I'll probably just push this far enough to get a full set of benchmark results so we can prioritize down the road, then leave it on ice until SN catches up.

@armanbilge
Copy link
Member
armanbilge commented Feb 10, 2025

Studied the implementation of ClassValue a bit, it still has some fairly complicated machinery. I wonder if we can beat it by doing something similar and simpler (I will prototype this later when I have time). The one definitive advantage that ClassValue has is that it's implemented by a field on Class.

@djspiewak
Copy link
Member Author
djspiewak commented Feb 12, 2025

The one definitive advantage that ClassValue has is that it's implemented by a field on Class.

Ostensibly it's also something the JIT understands. I think that's the real advantage, but we would need to actually examine the hotspot outputs to be sure.

@armanbilge
Copy link
Member
armanbilge commented Feb 16, 2025

I wonder if we can beat it by doing something similar and simpler (I will prototype this later when I have time).

I made an experimental "ClassByteMap" in 1472f3d, seems to be faster than ClassValue:

Benchmark                   Mode  Cnt         Score        Error  Units
TagBenchmark.classByteMap  thrpt   20  10417078.521 ± 466789.955  ops/s
TagBenchmark.classValue    thrpt   20   8920919.389 ± 110746.064  ops/s
TagBenchmark.tag           thrpt   20   7082499.380 ± 132528.920  ops/s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Cirrus JVM 🍄 enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

use java.lang.ClassValue instead of IO.tag

2 participants

@djspiewak @armanbilge
0