Conversation
will
commented
Apr 19, 2022
|
|
||
| if c.name == response | ||
| confirmed = true | ||
| self.confirmed = true |
Collaborator
Author
There was a problem hiding this comment.
This was a proper find, before it was just setting a local variable
will
commented
Apr 19, 2022
|
|
||
| if c.name == response | ||
| confirmed = true | ||
| self.confirmed = true |
will
commented
Apr 19, 2022
| end | ||
|
|
||
| to_run = checks.uniq.sort_by(&.name) | ||
| to_run = checks.uniq.sort_by!(&.name) |
Collaborator
Author
There was a problem hiding this comment.
I didn't know this one
src/cb/scope.cr:32:26 [Correctable]
[C] Performance/ChainedCallWithNoBang: Use bang method variant `sort_by!` after chained `uniq` call
> to_run = checks.uniq.sort_by(&.name)
^-----^
/tmp> cat a.cr
require "benchmark"
arr = [2,4,-5,2]
Benchmark.ips do |x|
x.report("orig") { arr.dup.uniq.sort_by(&.abs)}
x.report("bang") { arr.dup.uniq.sort_by!(&.abs)}
end
/tmp> crystal run --release a.cr
orig 7.04M (141.99ns) (± 2.24%) 224B/op 1.19× slower
bang 8.39M (119.19ns) (± 5.55%) 176B/op fastest
uses less memory and is faster. It makes sense because before it'd make an array for the uniq then yet another array for the sort, and with ! it just mutates the one it made for uniq. But that did not occur to me.
(also I added Benchmark.ips to crystal stdlib so it's extra fun to use 😎)
Member
|
WFM. Gotta start somewhere. 👍 |
abrightwell
approved these changes
Apr 19, 2022
will
added a commit
that referenced
this pull request
Oct 31, 2025
``` <<< /nix/store/6sk9jhxaqa01jm0wp0b4x9labcyi5a3c-cb-3.6.7.drv >>> /nix/store/jwxavs1iq4zp0bshivkz508js1s8f6g2-cb-3.6.7.drv Version changes: [C.] #1 binutils-with-gold 2.44.tar.bz2 x2 -> 2.44.tar.bz2 [C.] #2 builder.pl <none> x3 -> <none> x2 [C.] #3 cctools-binutils-darwin-wrapper 1010.6 x7 -> 1010.6 x5 [C.] #4 clang 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #5 clang-at-least 16-LLVMgold-path.patch x4 -> 16-LLVMgold-path.patch x3 [C.] #6 clang-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #7 clang-wrapper 20.1.8 x3, 21.1.2 x3 -> 21.1.2 x3 [C.] #8 compiler-rt 20.1.8, 21.1.2 -> 21.1.2 [C.] #9 compiler-rt-libc 20.1.8, 21.1.2 -> 21.1.2 [C.] #10 compiler-rt-src 20.1.8, 21.1.2 -> 21.1.2 [C.] #11 cpio 2.15 x3, 2.15.tar.bz2 x2 -> 2.15 x2, 2.15.tar.bz2 [C*] #12 crystal 1.10.1-1-darwin-universal.tar.gz, 1.16.3 -> 1.10.1-1-darwin-universal.tar.gz, 1.18.2 [C.] #13 expand-response-params <none> x5 -> <none> x4 [C.] #14 gnu-install-dirs.patch <none> x4 -> <none> x3 [C.] #15 jq 1.8.1 x2, 1.8.1.tar.gz x2 -> 1.8.1, 1.8.1.tar.gz [C.] #16 libbfd-plugin-api-header <none> x3 -> <none> x2 [C.] #17 libcxx 19.1.2+apple-sdk-15.5 x5 -> 19.1.2+apple-sdk-15.5 x4 [C.] #18 llvm 18-compatibility.patch, 20.1.8, 21.1.2 x2 -> 18-compatibility.patch, 21.1.2 x2 [C.] #19 llvm-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #20 llvm-tblgen 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #21 llvm-tblgen-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #22 macOS-SDK 11.3 x2, 14.4, 15.5 x3 -> 11.3 x2, 14.4, 15.5 x2 [C.] #23 make-binary-wrapper-hook <none> x4 -> <none> x3 [C.] #24 onig 6.9.10.tar.gz x2 -> 6.9.10.tar.gz [C.] #25 oniguruma 6.9.10 x2 -> 6.9.10 [C.] #26 pbzx 1.0.2 x3 -> 1.0.2 x2 [C.] #27 psutil 7.1.0.tar.gz x2 -> 7.1.0.tar.gz [C.] #28 python3 3.13.8 x2, 3.13.8-env x2 -> 3.13.8 x2, 3.13.8-env [C.] #29 python3.13-psutil 7.1.0 x2 -> 7.1.0 [C*] #30 source <none> x85 -> <none> x83 [C*] #31 stdenv-darwin <none> x5 -> <none> x3 Removed packages: [R.] #1 bd49bbaaafc98433a2cb4e95ce25b7a201baf5a5.patch <none> Closure size: 1239 -> 1201 (806 paths added, 844 paths removed, delta -38, disk usage -161.7KiB). ```
will
added a commit
that referenced
this pull request
Oct 31, 2025
* Allow temp location to change if necessary To keep the specs completely isolated, we don't want to reuse the same global tempdir. * update crystal 1.16 -> 1.18 ``` <<< /nix/store/6sk9jhxaqa01jm0wp0b4x9labcyi5a3c-cb-3.6.7.drv >>> /nix/store/jwxavs1iq4zp0bshivkz508js1s8f6g2-cb-3.6.7.drv Version changes: [C.] #1 binutils-with-gold 2.44.tar.bz2 x2 -> 2.44.tar.bz2 [C.] #2 builder.pl <none> x3 -> <none> x2 [C.] #3 cctools-binutils-darwin-wrapper 1010.6 x7 -> 1010.6 x5 [C.] #4 clang 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #5 clang-at-least 16-LLVMgold-path.patch x4 -> 16-LLVMgold-path.patch x3 [C.] #6 clang-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #7 clang-wrapper 20.1.8 x3, 21.1.2 x3 -> 21.1.2 x3 [C.] #8 compiler-rt 20.1.8, 21.1.2 -> 21.1.2 [C.] #9 compiler-rt-libc 20.1.8, 21.1.2 -> 21.1.2 [C.] #10 compiler-rt-src 20.1.8, 21.1.2 -> 21.1.2 [C.] #11 cpio 2.15 x3, 2.15.tar.bz2 x2 -> 2.15 x2, 2.15.tar.bz2 [C*] #12 crystal 1.10.1-1-darwin-universal.tar.gz, 1.16.3 -> 1.10.1-1-darwin-universal.tar.gz, 1.18.2 [C.] #13 expand-response-params <none> x5 -> <none> x4 [C.] #14 gnu-install-dirs.patch <none> x4 -> <none> x3 [C.] #15 jq 1.8.1 x2, 1.8.1.tar.gz x2 -> 1.8.1, 1.8.1.tar.gz [C.] #16 libbfd-plugin-api-header <none> x3 -> <none> x2 [C.] #17 libcxx 19.1.2+apple-sdk-15.5 x5 -> 19.1.2+apple-sdk-15.5 x4 [C.] #18 llvm 18-compatibility.patch, 20.1.8, 21.1.2 x2 -> 18-compatibility.patch, 21.1.2 x2 [C.] #19 llvm-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #20 llvm-tblgen 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #21 llvm-tblgen-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #22 macOS-SDK 11.3 x2, 14.4, 15.5 x3 -> 11.3 x2, 14.4, 15.5 x2 [C.] #23 make-binary-wrapper-hook <none> x4 -> <none> x3 [C.] #24 onig 6.9.10.tar.gz x2 -> 6.9.10.tar.gz [C.] #25 oniguruma 6.9.10 x2 -> 6.9.10 [C.] #26 pbzx 1.0.2 x3 -> 1.0.2 x2 [C.] #27 psutil 7.1.0.tar.gz x2 -> 7.1.0.tar.gz [C.] #28 python3 3.13.8 x2, 3.13.8-env x2 -> 3.13.8 x2, 3.13.8-env [C.] #29 python3.13-psutil 7.1.0 x2 -> 7.1.0 [C*] #30 source <none> x85 -> <none> x83 [C*] #31 stdenv-darwin <none> x5 -> <none> x3 Removed packages: [R.] #1 bd49bbaaafc98433a2cb4e95ce25b7a201baf5a5.patch <none> Closure size: 1239 -> 1201 (806 paths added, 844 paths removed, delta -38, disk usage -161.7KiB). ``` --------- Co-authored-by: Will Leinweber <will@bitfission.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This is a style enforcing linter like standardrb, etc. I don’t have that strong opinions on this either way, but the last PR there was some style discussion and it might be easy to defer to this?