-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
std.StaticStringMap: bump eval branch quota #19823
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
Conversation
Since the KVs are sorted, a linearly-growing bound will never be sufficient for extreme cases. Perhaps grow proportional to n^2 or something? |
52df5d0
to
eb162bd
Compare
Thanks, sounds good! Checking if test-std passes after changing quota to (10 * N * N) where N = kvs_list.len. |
eb162bd
to
8be5c5a
Compare
Done. |
8be5c5a
to
58321b7
Compare
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.
N squared??? NOT OK
If StaticStringMap was recently changed to be N^2 then it needs to be reverted pronto
The comptime initialization has not changed significantly from the original ComptimeStringMap impl. The limit was changed from 1500 to 30*N. But this was found to be insufficient for adversarial key arrangements. The N^2 limit is needed for sorting those keys which may require N^2 steps. |
Sorting should be O(N * log2(N)). Related: #13642 |
Sounds good. I will update the limit to C * N * log2(N). |
Great. I love that it documents the compilation time complexity. |
closes ziglang#19803 by changing quota from (30 * N) to (10 * N * log2(N)) where N = kvs_list.len * adds reported adversarial test case * update doc comment of getLongestPrefix()
58321b7
to
c8795e1
Compare
Just pushed a change. Now using |
Thanks! |
closes #19803 by changing quota from (30 * N) to (10 * N * log2(N)) where
N = kvs_list.len
EDITED