Skip to content

Unchecked page access #887

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ibraheemdev
Copy link
Contributor

Interested to see the performance results...

Copy link

netlify bot commented May 29, 2025

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit 950eb1a
🔍 Latest deploy log https://app.netlify.com/projects/salsa-rs/deploys/683a508314b9ff000869ac99

Copy link

codspeed-hq bot commented May 29, 2025

CodSpeed Performance Report

Merging #887 will improve performances by 16.45%

Comparing ibraheemdev:ibraheem/page-access (950eb1a) with master (8aaeb70)

Summary

⚡ 1 improvements
✅ 11 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
many_tracked_structs 37.8 µs 32.5 µs +16.45%

@ibraheemdev ibraheemdev force-pushed the ibraheem/page-access branch from 4687e94 to 950eb1a Compare May 31, 2025 00:42
@Veykril
Copy link
Member

Veykril commented Jun 11, 2025

I assume this is not doable for the same reason we cannot ever omit the type checks, someone could smuggle an index from one database to another. The type checks are a lot more expensive, I'd love if we could get rid of them

@ibraheemdev
Copy link
Contributor Author

Maybe an (unsound) feature flag with the invariant that your application only has a single database? Or maybe we could even have a global that panics if you try to create multiple databases?

@Veykril
Copy link
Member

Veykril commented Jun 12, 2025

That (panic on DB creation) does sound like a possible escape, that should also allow dropping the nonce check in that situation I think? Testing that seems a bit iffy though.

Now I wonder if there is a design where we could brand databases with an invariant lifetime given we already have a lifetime infestation everywhere anyways (though that one is currently covariant due to being the references lifetime). That could prevent smuggling as well? Though that won't avoid the nonce check, given the static caching system (I am not a fan of this honestly, but I can't think of a better way either)

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.

2 participants