Skip to content
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

[useId] Trade random collisions for collisions on overflow #29781

Merged
merged 3 commits into from
Nov 25, 2021

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Nov 20, 2021

Previously ids were picked at random from 1-1.000.000.000 (1e9). Naively one could assume that the probability of a collision if two numbers are drawn is 1e9 * 1e9. However, the birthday problem puts the probability at ~1e9 (see "Probability of a shared birthday").

The problem is that we fairly quickly reach a point were collisions are pretty likely e.g. with 200 random ids on a page (conceivable with inputs and tooltips on normal pages or even more problematic in our documentation) we have a 1:50.000 collision chance. https://instacalc.com/28845 for experimentation.

Instead, we can just increment and delay collisions to the point of integer overflow (at which point we almost certainly had a collision anyway)

@eps1lon eps1lon added bug 🐛 Something doesn't work package: utils Specific to the @mui/utils package labels Nov 20, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Nov 20, 2021

Details of bundle changes

Generated by 🚫 dangerJS against a5b774f

@eps1lon eps1lon marked this pull request as ready for review November 20, 2021 18:38
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 23, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 23, 2021
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 25, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 25, 2021
@eps1lon eps1lon merged commit febe13a into mui:master Nov 25, 2021
@eps1lon eps1lon deleted the fix/useId/birthday branch November 25, 2021 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work package: utils Specific to the @mui/utils package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants