Fixed format for 'dbg' #6241
Merged
Fixed format for 'dbg' #6241
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
piotr-3-janiszewski
changed the title
piotr-3-janiszewski
changed the title
Signed-off-by: Richard Feldman <[email protected]>
Signed-off-by: Rod <[email protected]>
Signed-off-by: Henrikh Kantuni <[email protected]>
Signed-off-by: Vladimir Zotov <[email protected]>
ankerl::dense_unordered is a very fast hash map that is built to be an index map. This enables extra optimizations compared to just wrapping a regular hash map. As such, I think this map is very well suited for our index map impl in Roc. I also think this dictionary implementation is simpler overall. On top of that, this removes the need for SIMD instructions for peak performance. Benchmarks of the C++ version and other C++ hash maps are here: https://martin.ankerl.com/2022/08/27/hashmap-bench-01/ Though this has clear bias of being written by the author of ankerl::dense_unordered, the results all look correct and the benchmarks thorough.
I think for now it is not worth considering adding hashflooding mitigation to the Roc Dict. 1. Wyhash is a secure enough has to pass SMHasher. 2. Languages like Go use wyhash in production and have not seen hashflooding issues. 3. We do have a random seed that Dicts use that changes per application linking. (Could monomorphize on Dict type for more randomness) 4. Any sort of fallback checks will lead to worse performance in the normal case. 5. A reasonable http server or app can limit the size of JSON or number of HTTP headers to accept.
Signed-off-by: Vladimir Zotov <[email protected]>
piotr-3-janiszewski
force-pushed
the
dbg_formatter_issue
branch
from
3c20e13
to
58b73ae
Compare
bhansconnect
approved these changes
Dec 11, 2023
|
Yes, there already has been a single line snapshost test, but because of the existence of "crates/compiler/test_syntax/tests/snapshots/pass/dbg.expr.formatted.roc" it was configured so that expanding the 'dbg' into multiple lines was considered correct. That is why I deleted that file. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've fixed the unconditional addition of new line after the 'dbg' keyword (issue #6188), and implemented the suggested way of formatting the 'dbg' with multi line condition.