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

Improve UDPTransport.sendto address validation (fixes: #214) #215

Merged
merged 1 commit into from
Jan 8, 2019

Conversation

jlaine
Copy link
Contributor

@jlaine jlaine commented Jan 8, 2019

When UDPTransport.sendto is called with a destination address, only
perform the validation once and store the result in an LRU cache. Also
store some of the socket's properties (family, proto, type) as integers
to avoid repeatedly fetching these properties and converting them to integers.

@@ -188,3 +182,17 @@ cdef class UDPTransport(UVBaseTransport):
# Ensure that what we buffer is immutable.
self.buffer.append((bytes(data), addr))
self._maybe_pause_protocol()

@functools.lru_cache()
def _validate_address(self, object addr):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try to make it a standalone function (not a method)? That way the cache will be shared across different transports.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, done.

When UDPTransport.sendto is called with a destination address, only
perform the validation once and store the result in an LRU cache. Also
store some of the socket's properties (family, proto, type) as integers
to avoid repeatedly fetching these properties and converting them to integers.
@jlaine jlaine force-pushed the udp-transport-sendto branch from 0016813 to ac85414 Compare January 8, 2019 23:06
@1st1 1st1 merged commit d5ad2b8 into MagicStack:master Jan 8, 2019
@1st1
Copy link
Member

1st1 commented Jan 8, 2019

Thank you!

@jlaine
Copy link
Contributor Author

jlaine commented Jan 8, 2019

Thanks for the amazingly fast turnaround!

@1st1
Copy link
Member

1st1 commented Jan 8, 2019

:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants