Clean up a hostinfo to reduce memory usage #955
Conversation
nbrownus
force-pushed
the
getOrHandshake
branch
from
3656e8c to
95da1f6
Compare
nbrownus
force-pushed
the
smaller-hostinfo
branch
from
88b2091 to
cb88583
Compare
| @@ -350,22 +340,6 @@ func ixHandshakeStage2(f *Interface, addr *udp.Addr, via *ViaSender, hostinfo *H | |||
| } | |||
|
|
|||
| ci := hostinfo.ConnectionState | |||
| if ci.ready { | |||
There was a problem hiding this comment.
if we remove this ci.ready check, I think we need a different double check after the hh.Lock() to see if a different thread already completed the handshake for this host? How do we protected about late handshake packets coming in?
There was a problem hiding this comment.
I expect in this version it would fail when trying to read a noise message. The next PR in the series would put these calls to ixHandshake* within the pending hostmap lock to avoid these races.
| hostinfo.Lock() | ||
| defer hostinfo.Unlock() |
There was a problem hiding this comment.
The race detector might get upset about this, even if its okay in theory. I guess we can see what happens.
There was a problem hiding this comment.
It looks like its fine, at least running make smoke-docker-race shows no race detector complaints during the smoke test.
| hh.Lock() | ||
| defer hh.Unlock() |
There was a problem hiding this comment.
I think everytime we do a lock like this, we need to double check afterwards if another thread beat us to finishing out this entry, right? (or add a comment about why no-one could have beat us to it).
| hostinfo.Lock() | ||
| defer hostinfo.Unlock() |
There was a problem hiding this comment.
Why don't we need this anymore? Shouldn't be at least doublecheck that someone else didn't already handle a recv_error for this hostinfo on a different thread?
There was a problem hiding this comment.
The lock didn't really do anything for us before. If we have two threads in this code path and 1 is temporarily blocked on the hostinfo lock, they will have the same outcome.
nbrownus
force-pushed
the
smaller-hostinfo
branch
from
ec9d8a4 to
0fee9a7
Compare
This removes a lot of state from
HostInfoto reduce our memory footprint for active tunnels.