-
Notifications
You must be signed in to change notification settings - Fork 656
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Refactored UInt32 and UInt64 methods nextPowerOf2 #371
Conversation
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
@swift-nio-bot test this please |
Sources/NIO/ByteBuffer-int.swift
Outdated
n += 1 | ||
|
||
return n | ||
return 1 << (64 - (self - 1).leadingZeroBitCount) |
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.
@adamnemecek ah nice, didn't know we had leadingZeroBitCount
, hope that gets lowered to the LZCNT
instruction.
You can actually get the number of bits a type has from Self.bitWidth
. Then the code for all unsigned integer types will actually be the same. You can probably just put the extension on UnsignedInteger
and you'll get it for all.
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 uses bsr https://godbolt.org/g/A9RjhW
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.
@adamnemecek awesome, thank you! TIL that godbolt supports Swift, how cool's that!
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.
You can guess who added the support. His name starts with A and ends with dam Nemecek.
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.
that's awesome, TIL about leadingZeroBitCount
. Just some minor nits
Sources/NIO/ByteBuffer-int.swift
Outdated
@@ -97,22 +97,10 @@ extension ByteBuffer { | |||
extension UInt64 { | |||
/// Returns the next power of two. | |||
public func nextPowerOf2() -> UInt64 { | |||
guard self > 0 else { | |||
if self == 0 { |
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.
I don't think we should take this change. Happy to make it guard self != 0 else { ... }
though. But the guard
tells/should tell the compiler that this is the less likely case and it can optimise for that.
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.
Looks great... thanks!
@swift-nio-bot test this please |
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, thank you!
@swift-nio-bot test this please |
@swift-nio-bot test this please |
Refactored nextPowerOf2 methods.
Motivation:
Make them shorter and somewhat faster.
Modifications:
Changed the implementations of nextPowerOf2
Result:
Methods are shorter.