Browse Source

Remove runtime.GC() call from server entry streaming
- Testing showed this call didn't change memory profile, but
did cause increased CPU usage

Rod Hynes 8 years ago
parent
commit
ab3ab0696b
2 changed files with 5 additions and 9 deletions
  1. 1 2
      psiphon/common/protocol/serverEntry.go
  2. 4 7
      psiphon/dataStore.go

+ 1 - 2
psiphon/common/protocol/serverEntry.go

@@ -242,8 +242,7 @@ func NewStreamingServerEntryDecoder(
 //   will allocate memory to hex decode and JSON deserialze the server
 //   entry. As this is not presently reusing a fixed buffer, each call
 //   will allocate additional memory; garbage collection is necessary to
-//   reclaim that memory for reuse for the next server entry. Memory-
-//   constrained users could call runtime.GC() after each call to Next.
+//   reclaim that memory for reuse for the next server entry.
 //
 func (decoder *StreamingServerEntryDecoder) Next() (*ServerEntry, error) {
 

+ 4 - 7
psiphon/dataStore.go

@@ -28,7 +28,6 @@ import (
 	"net"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 	"sync"
 	"time"
@@ -277,6 +276,10 @@ func StreamingStoreServerEntries(
 
 	checkInitDataStore()
 
+	// Note: both StreamingServerEntryDecoder.Next and StoreServerEntry
+	// allocate temporary memory buffers for hex/JSON decoding/encoding,
+	// so this isn't true constant-memory streaming (it depends on garbage
+	// collection).
 	for {
 		serverEntry, err := serverEntries.Next()
 		if err != nil {
@@ -292,12 +295,6 @@ func StreamingStoreServerEntries(
 		if err != nil {
 			return common.ContextError(err)
 		}
-
-		// Both StreamingServerEntryDecoder.Next and StoreServerEntry allocate
-		// memory. To approximate true fixed-memory streaming, garbage collect
-		// to reclaim that memory for the next iteration.
-		// TODO: measure effectiveness and performance penalty of this call
-		runtime.GC()
 	}
 
 	// Since there has possibly been a significant change in the server entries,