Merge pull request #5 from getlantern/optimize-for-mobile

Reduce allocations and GC pressure for mobile devices

Author: myleshorton

bench_test.go

@@ -9,7 +9,7 @@ import (
 )
 
 func BenchmarkFrontPoolTakeReturn(b *testing.B) {
-	pool := newFrontPool()
+	pool := newFrontPool(0)
 	f := newFront(&Masquerade{Domain: "cdn.example.com", IpAddress: "1.2.3.4"}, "test")
 	f.markSucceeded()
 	pool.addReady(f)
@@ -23,7 +23,7 @@ func BenchmarkFrontPoolTakeReturn(b *testing.B) {
 }
 
 func BenchmarkFrontPoolCandidates(b *testing.B) {
-	pool := newFrontPool()
+	pool := newFrontPool(0)
 	fronts := make([]*front, 5000)
 	for i := range fronts {
 		fronts[i] = newFront(&Masquerade{
@@ -47,7 +47,7 @@ func BenchmarkRewriteRequest(b *testing.B) {
 	req.Header.Set("X-Custom", "value")
 	b.ResetTimer()
 	for b.Loop() {
-		_, _ = rewriteRequest(req, "d1234.cloudfront.net", nil)
+		_ = rewriteRequest(req, "d1234.cloudfront.net", nil)
 	}
 }
 
@@ -122,6 +122,22 @@ func BenchmarkFrontsToCache(b *testing.B) {
 	}
 }
 
+// BenchmarkFrontsToCacheRealistic simulates a typical pool where only ~10%
+// of fronts have been tested and succeeded — the common case on mobile.
+func BenchmarkFrontsToCacheRealistic(b *testing.B) {
+	fronts := make([]*front, 1000)
+	for i := range fronts {
+		fronts[i] = newFront(&Masquerade{Domain: "cdn.example.com", IpAddress: "1.2.3.4"}, "test")
+		if i < 100 { // only 10% succeeded
+			fronts[i].markSucceeded()
+		}
+	}
+	b.ResetTimer()
+	for b.Loop() {
+		_ = frontsToCache(fronts, 1000)
+	}
+}
+
 func BenchmarkGoroutineCount(b *testing.B) {
 	// Measure goroutine overhead of creating and closing a client
 	config := &Config{

cache.go

@@ -53,18 +53,24 @@ func (NopCache) Load() ([]*CachedFront, error) { return nil, nil }
 func (NopCache) Save([]*CachedFront) error      { return nil }
 
 // frontsToCache converts pool fronts to cache format, limited to maxSize entries.
+// Only fronts that have previously succeeded are cached — this avoids allocating
+// CachedFront structs for the (typically much larger) set of untested fronts.
 func frontsToCache(fronts []*front, maxSize int) []*CachedFront {
-	if len(fronts) > maxSize {
-		fronts = fronts[:maxSize]
-	}
-	cached := make([]*CachedFront, 0, len(fronts))
+	cached := make([]*CachedFront, 0, min(len(fronts), maxSize))
 	for _, f := range fronts {
+		ts := f.lastSucceededTime()
+		if ts.IsZero() {
+			continue // never succeeded, skip
+		}
+		if len(cached) >= maxSize {
+			break
+		}
 		cached = append(cached, &CachedFront{
 			Domain:         f.Domain,
 			IpAddress:      f.IpAddress,
 			SNI:            f.SNI,
 			VerifyHostname: f.VerifyHostname,
-			LastSucceeded:  f.lastSucceededTime(),
+			LastSucceeded:  ts,
 			ProviderID:     f.ProviderID,
 		})
 	}

cache_test.go

@@ -74,9 +74,20 @@ func TestFrontsToCache(t *testing.T) {
 		newFront(&Masquerade{Domain: "b.com", IpAddress: "2.2.2.2"}, "p2"),
 		newFront(&Masquerade{Domain: "c.com", IpAddress: "3.3.3.3"}, "p3"),
 	}
+	// Mark all as succeeded — only succeeded fronts are cached
+	for _, f := range fronts {
+		f.markSucceeded()
+	}
 
 	cached := frontsToCache(fronts, 2)
-	assert.Len(t, cached, 2)
+	assert.Len(t, cached, 2, "should be limited to maxSize")
+
+	// Verify that never-succeeded fronts are skipped
+	unsucceeded := []*front{
+		newFront(&Masquerade{Domain: "d.com", IpAddress: "4.4.4.4"}, "p4"),
+	}
+	cached = frontsToCache(unsucceeded, 10)
+	assert.Len(t, cached, 0, "never-succeeded fronts should not be cached")
 }
 
 func TestFileCache_CreatesDirs(t *testing.T) {

config.go

@@ -52,10 +52,17 @@ type Masquerade struct {
 // Lookup returns the fronted hostname for the given origin hostname.
 // Returns empty string if the provider has no mapping for the host.
 func (p *Provider) Lookup(hostname string) string {
-	if h, _, err := net.SplitHostPort(hostname); err == nil {
-		hostname = h
+	// Strip port if present. Check for colon first to avoid net.SplitHostPort
+	// which allocates a *AddrError for port-less hostnames (the common case).
+	if strings.LastIndexByte(hostname, ':') >= 0 {
+		if h, _, err := net.SplitHostPort(hostname); err == nil {
+			hostname = h
+		}
 	}
-	hostname = strings.ToLower(hostname)
+	// Only allocate a lowercase copy when the hostname isn't already lowercase.
+	// In practice, hostnames from Android/Go HTTP clients are almost always
+	// lowercase, so this avoids an allocation on the hot request path.
+	hostname = toLowerFast(hostname)
 
 	if alias := p.HostAliases[hostname]; alias != "" {
 		return alias
@@ -71,6 +78,16 @@ func (p *Provider) Lookup(hostname string) string {
 	return ""
 }
 
+// toLowerFast returns s lowercased, reusing s if it's already all-lowercase.
+func toLowerFast(s string) string {
+	for i := range s {
+		if s[i] >= 'A' && s[i] <= 'Z' {
+			return strings.ToLower(s)
+		}
+	}
+	return s
+}
+
 // ParseConfig parses a gzipped YAML configuration into a Config.
 func ParseConfig(gzippedYaml []byte) (*Config, error) {
 	r, err := gzip.NewReader(bytes.NewReader(gzippedYaml))

dialer.go

@@ -36,31 +36,13 @@ type dialResult struct {
 }
 
 // dialFront performs a TLS connection to the given front.
+// Builds the tls.Config in a single allocation (no Clone).
 func dialFront(ctx context.Context, f *front, rootCAs *x509.CertPool, clientHelloID tls.ClientHelloID, dialer Dialer) dialResult {
 	addr := f.IpAddress
 	if _, _, err := net.SplitHostPort(addr); err != nil {
 		addr = net.JoinHostPort(addr, "443")
 	}
 
-	tlsConfig := &tls.Config{
-		ServerName: f.Domain,
-		RootCAs:    rootCAs,
-	}
-
-	var sendServerNameExtension bool
-	if f.SNI != "" {
-		sendServerNameExtension = true
-		tlsConfig.ServerName = f.SNI
-		tlsConfig.InsecureSkipVerify = true
-		tlsConfig.VerifyPeerCertificate = func(rawCerts [][]byte, _ [][]*x509.Certificate) error {
-			var verifyHostname string
-			if f.VerifyHostname != nil {
-				verifyHostname = *f.VerifyHostname
-			}
-			return verifyPeerCertificate(rawCerts, rootCAs, verifyHostname)
-		}
-	}
-
 	dialCtx, dialCancel := context.WithTimeout(ctx, dialTimeout)
 	defer dialCancel()
 
@@ -69,18 +51,31 @@ func dialFront(ctx context.Context, f *front, rootCAs *x509.CertPool, clientHell
 		return dialResult{nil, classifyError(err), err}
 	}
 
-	configCopy := tlsConfig.Clone()
-	configCopy.InsecureSkipVerify = true
-	if !sendServerNameExtension {
-		configCopy.ServerName = ""
+	// Build final tls.Config directly — single allocation instead of alloc + Clone.
+	config := &tls.Config{
+		RootCAs:            rootCAs,
+		InsecureSkipVerify: true,
+	}
+
+	useSNI := f.SNI != ""
+	if useSNI {
+		config.ServerName = f.SNI
+		config.VerifyPeerCertificate = func(rawCerts [][]byte, _ [][]*x509.Certificate) error {
+			var verifyHostname string
+			if f.VerifyHostname != nil {
+				verifyHostname = *f.VerifyHostname
+			}
+			return verifyPeerCertificate(rawCerts, rootCAs, verifyHostname)
+		}
 	}
+	// When not using SNI, ServerName stays empty so no SNI extension is sent.
 
 	chid := clientHelloID
 	if chid.Client == "" {
 		chid = tls.HelloGolang
 	}
 
-	conn := tls.UClient(rawConn, configCopy, chid)
+	conn := tls.UClient(rawConn, config, chid)
 	deadline := time.Now().Add(dialTimeout)
 	rawConn.SetDeadline(deadline)
 
@@ -91,13 +86,13 @@ func dialFront(ctx context.Context, f *front, rootCAs *x509.CertPool, clientHell
 	rawConn.SetDeadline(time.Time{})
 
 	// For non-SNI case, verify the cert manually after handshake
-	if !tlsConfig.InsecureSkipVerify {
+	if !useSNI {
 		state := conn.ConnectionState()
 		rawCerts := make([][]byte, len(state.PeerCertificates))
 		for i, cert := range state.PeerCertificates {
 			rawCerts[i] = cert.Raw
 		}
-		if err := verifyPeerCertificate(rawCerts, tlsConfig.RootCAs, tlsConfig.ServerName); err != nil {
+		if err := verifyPeerCertificate(rawCerts, rootCAs, f.Domain); err != nil {
 			rawConn.Close()
 			return dialResult{nil, false, err}
 		}

domainfront.go

@@ -13,7 +13,6 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
-	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -60,6 +59,7 @@ type Client struct {
 	httpClient *http.Client
 
 	crawlerConcurrency int
+	readyQueueSize     int
 	wg                 sync.WaitGroup
 }
 
@@ -79,6 +79,16 @@ func WithClientHelloID(id tls.ClientHelloID) Option {
 }
 func WithCrawlerConcurrency(n int) Option { return func(c *Client) { c.crawlerConcurrency = n } }
 
+// WithReadyQueueSize sets the capacity of the ready-fronts channel.
+// Smaller values save memory on constrained devices. Default is 500.
+func WithReadyQueueSize(n int) Option { return func(c *Client) { c.readyQueueSize = n } }
+
+// WithCacheSaveInterval sets how often dirty cache state is flushed to disk.
+// Longer intervals reduce I/O on flash storage (e.g. Android). Default is 5s.
+func WithCacheSaveInterval(d time.Duration) Option {
+	return func(c *Client) { c.cacheSaveInterval = d }
+}
+
 // WithCacheFile is a convenience option that sets a FileCache at the given path.
 func WithCacheFile(path string) Option {
 	return func(c *Client) { c.cache = &FileCache{Path: path} }
@@ -97,7 +107,7 @@ func New(ctx context.Context, config *Config, options ...Option) (*Client, error
 		ctx:                innerCtx,
 		cancel:             cancel,
 		log:                slog.Default(),
-		pool:               newFrontPool(),
+		pool:               nil, // created after options are applied
 		providers:          make(map[string]*Provider),
 		dialer:             NetDialer{},
 		clientHelloID:      tls.HelloChrome_131,
@@ -121,6 +131,9 @@ func New(ctx context.Context, config *Config, options ...Option) (*Client, error
 		c.crawlerConcurrency = 1
 	}
 
+	// Create pool after options are applied so readyQueueSize can be set
+	c.pool = newFrontPool(c.readyQueueSize)
+
 	if err := c.applyConfig(config); err != nil {
 		cancel()
 		return nil, fmt.Errorf("invalid config: %w", err)
@@ -230,15 +243,26 @@ func (c *Client) notifyCacheDirty() {
 func (c *Client) crawler() {
 	defer c.wg.Done()
 
+	// Use a reusable timer instead of time.After to avoid leaking timers.
+	// time.After creates a new timer each iteration that isn't GC'd until it
+	// fires — on memory-constrained devices this creates mounting GC pressure.
+	// Initialize stopped and drain to avoid an immediate spurious wake.
+	timer := time.NewTimer(0)
+	if !timer.Stop() {
+		<-timer.C
+	}
+	defer timer.Stop()
+
 	for {
 		if c.pool.readyCount() < 2 {
 			c.crawlAllFronts()
 		}
 
+		timer.Reset(time.Duration(6+rand.IntN(7)) * time.Second)
 		select {
 		case <-c.ctx.Done():
 			return
-		case <-time.After(time.Duration(6+rand.IntN(7)) * time.Second):
+		case <-timer.C:
 		}
 	}
 }
@@ -300,16 +324,19 @@ func (c *Client) vetFront(f *front) bool {
 	return c.verifyWithPost(result.conn, provider.TestURL)
 }
 
+// vetBody is reused across vet requests to avoid per-call allocation.
+var vetBody = []byte("a")
+
 func (c *Client) verifyWithPost(conn net.Conn, testURL string) bool {
 	tr := newConnTransport(conn, true)
-	client := &http.Client{Transport: tr}
-	req, err := http.NewRequest(http.MethodPost, testURL, strings.NewReader("a"))
+	req, err := http.NewRequest(http.MethodPost, testURL, bytes.NewReader(vetBody))
 	if err != nil {
 		c.log.Debug("Error creating vet request", "error", err)
 		return false
 	}
+	req.URL.Scheme = "http" // TLS already established on conn
 	req.Header.Set("Content-Type", "application/json")
-	resp, err := client.Do(req)
+	resp, err := tr.RoundTrip(req)
 	if err != nil {
 		c.log.Debug("Error vetting front", "error", err, "url", testURL)
 		return false
@@ -328,16 +355,20 @@ func (c *Client) verifyWithPost(conn net.Conn, testURL string) bool {
 func (c *Client) cacheSaver() {
 	defer c.wg.Done()
 
+	timer := time.NewTimer(c.cacheSaveInterval)
+	defer timer.Stop()
+
 	for {
 		select {
 		case <-c.ctx.Done():
 			return
-		case <-time.After(c.cacheSaveInterval):
+		case <-timer.C:
 			select {
 			case <-c.cacheDirty:
 				c.saveCache()
 			default:
 			}
+			timer.Reset(c.cacheSaveInterval)
 		}
 	}
 }