Skip to content
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

make the ping queue use a more compact data structure #16

Merged
merged 2 commits into from Aug 14, 2016

Conversation

@arvidn
Copy link
Contributor

arvidn commented Aug 14, 2016

and reduce some IPv4/IPv6 code duplication.

@@ -390,19 +390,6 @@ struct bound_socket
char packet[1500];
};

// this is the type of each node queued up
// to be pinged at some point in the future
struct queued_node_t

This comment has been minimized.

@arvidn

arvidn Aug 14, 2016 Author Contributor

I moved this down a bit and stripped out fields that aren't used externally

int sock_idx;
};

template <typename Address>
struct ping_queue_t

This comment has been minimized.

@arvidn

arvidn Aug 14, 2016 Author Contributor

while working on this increasingly more fields started having an IPv4 and IPv6 version, so making it a template made that a lot cleaner. I also moved in the queue entry type as an inner class

main.cpp Outdated
std::uint32_t sock_idx:2;

// expiration in seconds (relative to the ping_queue creation time)
std::uint32_t expire:30;

This comment has been minimized.

@arvidn

arvidn Aug 14, 2016 Author Contributor

I suppose technically there will always be 2 bytes padding here, so the bitfield may be a bit redundant

This comment has been minimized.

@ssiloti

ssiloti Aug 14, 2016 Contributor

Yeah, as-is the bitfield is just generating slower code for no gain.

@ssiloti
Copy link
Contributor

ssiloti commented Aug 14, 2016

lgtm

@arvidn arvidn merged commit 44e19b0 into bittorrent:master Aug 14, 2016
@arvidn arvidn deleted the arvidn:compact-ping-queue branch Aug 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.