feat: create Smart Dialer #185
Conversation
x/mobileproxy/mobileproxy.go
Outdated
| StreamDialer: &transport.TCPDialer{}, | ||
| PacketDialer: &transport.UDPDialer{}, | ||
| } | ||
| testDomainsSlice := append(make([]string, 0, len(testDomains.list)), testDomains.list...) |
There was a problem hiding this comment.
question Why not using testDomain.list here? To make a deep copy of the testDomains because StringList is still mutable to the caller?
There was a problem hiding this comment.
Good point. I've iterated on this code a lot, and I think at one point I was holding on to the list for later. There's no point in doing that now, since we don't refer to the list after NewDialer returns. I'm using your suggestion now
|
|
||
| go func(entry E, testDone context.CancelFunc) { | ||
| defer testDone() | ||
| result, err := test(entry) |
There was a problem hiding this comment.
Maybe not in this PR, test can accept a ctx object to it knows when to cancel the request. Because there might be resource leak here:
- go func () { test(entries[0]); }
- go func() { test(entries[1]); }
- test(entries[0]) finished with
nil-error, returns - If test(entries[1]) blocks, there are resource leaks
There was a problem hiding this comment.
It's strictly not necessary. I would just be forwarding the input context. If you are calling raceTests, you are also providing the context and function, so you can write the input function in a way that uses the input context.
That's exactly what I do in the stream_dialer.go code:
resolver, err := raceTests(ctx, 250*time.Millisecond, resolvers, func(resolver *smartResolver) (*smartResolver, error) {
for _, testDomain := range testDomains {
select {
case <-ctx.Done():
return nil, ctx.Err()
default:
}
...
Co-authored-by: J. Yi <93548144+jyyi1@users.noreply.github.com>
Co-authored-by: J. Yi <93548144+jyyi1@users.noreply.github.com>
There was a problem hiding this comment.
FYI, I added a context to the DNS and TLS searches. I also added logCtx to make sure we don't output logs after the search is done, which is confusing.
I'm going to submit since you are out today and I'm going on vacation, but feel free to comment and I'll address any issue when I'm back.
|
@fortuna Sounds good. One thing I just thought of is, maybe we can use |
|
I considered that. While the move to front operation is O(1), you still
need to scan the list. Scanning an array twice is probably faster than
scanning a linked list once.
We would have to combine the linked list with a map. It’s doable, but given
the size of the list (100), I decided to punt on it, as the time is minimum
compared with the network time.
…On Thu, Feb 22, 2024 at 6:50 PM J. Yi ***@***.***> wrote:
@fortuna <https://github.com/fortuna> Sounds good. One thing I just
thought of is, maybe we can use container/list
<https://pkg.go.dev/container/list> to implement the LRU cache since it
might (at least in theory) be more efficient.
—
Reply to this email directly, view it on GitHub
<#185 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA3XHOGUJI7Y4XRVIGXFDTYU7KSTAVCNFSM6AAAAABDR5XRYOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRQGUZDMNBQHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Indeed, that's why I added "in theory". Scanning a continuous array is definitely faster than scanning the linked-list pointers scattered in the memory. |
Preliminary code for the Smart Dialer and accompanying strategy finding.
I believe there's a lot to improve, but I'd rather move this to main sooner rather than later and iterate.
Sample output of the smart-proxy example app, with traffic going over a proxy in Russia:
This is an example of running it in Iran: