-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
docs: add time complexity to relevant primops #14554
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
base: master
Are you sure you want to change the base?
Conversation
roberth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome.
I've added suggestions to most of them. Maybe not consistently; apologies for that.
| # Time Complexity | ||
| O(n * T_f) where: | ||
| n = requested length | ||
| T_f = generator function evaluation time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't call them, so time complexity is O(n)
nix-repl> builtins.length (builtins.genList (x: throw "nope") 1000)
1000| O(n * T_f) where: | ||
| n = list length | ||
| T_f = function evaluation time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| O(n * T_f) where: | |
| n = list length | |
| T_f = function evaluation time | |
| O(n) where: | |
| n = list length | |
| Calls to `f` are performed afterwards, when needed. |
| O(n * T_f) where: | ||
| n = requested length | ||
| T_f = generator function evaluation time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| O(n * T_f) where: | |
| n = requested length | |
| T_f = generator function evaluation time | |
| O(n) where: | |
| n = requested length | |
| Calls to `generator` are performed afterwards when needed. |
| - O(n) best case (attribute set already sorted) | ||
| - O(n log n) worst case (requires sorting), where: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no user-facing concept of attrset sortedness, and I don't think the best case is implemented? Usually that'd be a special case in the sorting algorithm which makes the average case worse.
Furthermore, I think pinning down the best case is too ambitious and not even useful to users.
| # Time Complexity | ||
| O(n) where n = total length of output string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically there's also the evaluation of each element, as this function is currently strict in those.
(I'm not sure that it will remain so! I've heard talk about lazy ropes)
| n = length of input string | ||
| k = number of replacement patterns | ||
| c = average length of patterns in 'from' list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Average actually works out ok here, because of the * k.
| O(n * log m) where: | ||
| n = number of sets in input list | ||
| m = average number of attributes per set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically not true for skewed distributions, I believe. We can keep it vague again; it's ok.
| m = average number of attributes per set | |
| m = number of attributes per set |
| n = number of attribute sets in input list | ||
| k = number of unique keys across all sets | ||
| More precisely: O(n * m * log k) where m ≤ k is average number of attributes per set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't checked this one.
| # Time Complexity | ||
| O(n) where n = list length (must copy n-1 elements) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| O(n) where n = list length (must copy n-1 elements) | |
| O(n) where n = list length (copies n-1 elements) |
Reason (not to be included): Could be changed perhaps if testing shows that a different data structure for large lists results in a speedup in practice.
This PR adds time complexity annotations (in Big O notation) to Nix builtin functions and operators.
providing users with the information needed to reason about evaluation performance.
Motivation
Broader impact
This was created working together with @adisbladis in Thaigersprint/thaigersprint-2025#2
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.