Skip to content

[fix] Use validate_maybe_provisional instead of validate_provisional#789

Merged
MichaReiser merged 1 commit intosalsa-rs:masterfrom
MichaReiser:use-maybe-provisional
Apr 9, 2025
Merged

[fix] Use validate_maybe_provisional instead of validate_provisional#789
MichaReiser merged 1 commit intosalsa-rs:masterfrom
MichaReiser:use-maybe-provisional

Conversation

@MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Apr 9, 2025

I accidentally changed the validate_maybe_provisional calls to validate_provisional in #788. This is bad for performance because it prevents Salsa from short-circuiting if the memo is already finalized.

@netlify
Copy link

netlify bot commented Apr 9, 2025

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit 21db86c
🔍 Latest deploy log https://app.netlify.com/sites/salsa-rs/deploys/67f6b70b0b16ac0008da4f32

@MichaReiser MichaReiser marked this pull request as ready for review April 9, 2025 18:07
@codspeed-hq
Copy link

codspeed-hq bot commented Apr 9, 2025

CodSpeed Performance Report

Merging #789 will not alter performance

Comparing MichaReiser:use-maybe-provisional (21db86c) with master (69272a8)

Summary

✅ 12 untouched benchmarks

@MichaReiser
Copy link
Contributor Author

I confirmed on the Red Knot repo that this fixes the 3% incremental perf regression

@MichaReiser MichaReiser added this pull request to the merge queue Apr 9, 2025
Merged via the queue into salsa-rs:master with commit b165ba7 Apr 9, 2025
11 checks passed
@MichaReiser MichaReiser deleted the use-maybe-provisional branch April 9, 2025 18:26
@github-actions github-actions bot mentioned this pull request Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant