Pull request: 4437 depr memory opt

Merge in DNS/adguard-home from 4437-rm-mem-opt to master

Updates #4437.
Updates #2044.

Squashed commit of the following:

commit d1e5520213f6b68570d18a8d831d4923112901ba
Merge: 73a6b494 8bb95469
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Apr 6 19:37:09 2022 +0300

    Merge branch 'master' into 4437-rm-mem-opt

commit 73a6b4948cb32f1cb79a54b244018b29382fad76
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Apr 6 18:33:23 2022 +0300

    all: imp log of changes

commit a62efcdcd44de300726c906c7f6198c0a02d4ccf
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Apr 6 18:27:42 2022 +0300

    home: depr memory opt

CHANGELOG.md

@@ -79,6 +79,8 @@ In this release, the schema version has changed from 12 to 13.
 
 ### Deprecated
 
+- Obsolete `--no-mem-optimization` option ([#4437]).  v0.109.0 will remove the
+  flag completely.
 - Go 1.17 support.  v0.109.0 will require at least Go 1.18 to build.
 
 ### Fixed
@@ -111,6 +113,7 @@ In this release, the schema version has changed from 12 to 13.
 [#4221]: https://github.com/AdguardTeam/AdGuardHome/issues/4221
 [#4238]: https://github.com/AdguardTeam/AdGuardHome/issues/4238
 [#4276]: https://github.com/AdguardTeam/AdGuardHome/issues/4276
+[#4437]: https://github.com/AdguardTeam/AdGuardHome/issues/4437
 
 [repr]:         https://reproducible-builds.org/docs/source-date-epoch/
 [doq-draft-10]: https://datatracker.ietf.org/doc/html/draft-ietf-dprive-dnsoquic-10#section-10.2

internal/home/home.go

@@ -389,9 +389,6 @@ func run(args options, clientBuildFS fs.FS) {
 	// configure log level and output
 	configureLogger(args)
 
-	// Go memory hacks
-	memoryUsage(args)
-
 	// Print the first message after logger is configured.
 	log.Println(version.Full())
 	log.Debug("current working directory is %s", Context.workDir)

internal/home/memory.go

@@ -1,40 +0,0 @@
-package home
-
-import (
-	"os"
-	"runtime/debug"
-	"time"
-
-	"github.com/AdguardTeam/golibs/log"
-)
-
-// memoryUsage implements a couple of not really beautiful hacks which purpose is to
-// make OS reclaim the memory freed by AdGuard Home as soon as possible.
-// See this for the details on the performance hits & gains:
-// https://github.com/AdguardTeam/AdGuardHome/internal/issues/2044#issuecomment-687042211
-func memoryUsage(args options) {
-	if args.disableMemoryOptimization {
-		log.Info("Memory optimization is disabled")
-		return
-	}
-
-	// Makes Go allocate heap at a slower pace
-	// By default we keep it at 50%
-	debug.SetGCPercent(50)
-
-	// madvdontneed: setting madvdontneed=1 will use MADV_DONTNEED
-	// instead of MADV_FREE on Linux when returning memory to the
-	// kernel. This is less efficient, but causes RSS numbers to drop
-	// more quickly.
-	_ = os.Setenv("GODEBUG", "madvdontneed=1")
-
-	// periodically call "debug.FreeOSMemory" so
-	// that the OS could reclaim the free memory
-	go func() {
-		ticker := time.NewTicker(5 * time.Minute)
-		for range ticker.C {
-			log.Debug("free os memory")
-			debug.FreeOSMemory()
-		}
-	}()
-}

internal/home/options.go

@@ -7,6 +7,7 @@ import (
 	"strconv"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/version"
+	"github.com/AdguardTeam/golibs/log"
 )
 
 // options passed from command-line arguments
@@ -27,10 +28,6 @@ type options struct {
 	// runningAsService flag is set to true when options are passed from the service runner
 	runningAsService bool
 
-	// disableMemoryOptimization - disables memory optimization hacks
-	// see memoryUsage() function for the details
-	disableMemoryOptimization bool
-
 	glinetMode bool // Activate GL-Inet compatibility mode
 
 	// noEtcHosts flag should be provided when /etc/hosts file shouldn't be
@@ -180,8 +177,12 @@ var noCheckUpdateArg = arg{
 var disableMemoryOptimizationArg = arg{
 	"Disable memory optimization.",
 	"no-mem-optimization", "",
-	nil, func(o options) (options, error) { o.disableMemoryOptimization = true; return o, nil }, nil,
+	nil, nil, func(_ options, _ string) (f effect, err error) {
-	func(o options) []string { return boolSliceOrNil(o.disableMemoryOptimization) },
+		log.Info("warning: using --no-mem-optimization flag has no effect and is deprecated")
+
+		return nil, nil
+	},
+	func(o options) []string { return nil },
 }
 
 var verboseArg = arg{

internal/home/options_test.go

@@ -101,9 +101,13 @@ func TestParseDisableUpdate(t *testing.T) {
 	assert.True(t, testParseOK(t, "--no-check-update").disableUpdate, "--no-check-update is disable update")
 }
 
+// TODO(e.burkov):  Remove after v0.108.0.
 func TestParseDisableMemoryOptimization(t *testing.T) {
-	assert.False(t, testParseOK(t).disableMemoryOptimization, "empty is not disable update")
+	o, eff, err := parse("", []string{"--no-mem-optimization"})
-	assert.True(t, testParseOK(t, "--no-mem-optimization").disableMemoryOptimization, "--no-mem-optimization is disable update")
+	require.NoError(t, err)
+
+	assert.Nil(t, eff)
+	assert.Zero(t, o)
 }
 
 func TestParseService(t *testing.T) {
@@ -127,8 +131,6 @@ func TestParseUnknown(t *testing.T) {
 }
 
 func TestSerialize(t *testing.T) {
-	const reportFmt = "expected %s but got %s"
-
 	testCases := []struct {
 		name string
 		opts options
@@ -173,19 +175,14 @@ func TestSerialize(t *testing.T) {
 		name: "glinet_mode",
 		opts: options{glinetMode: true},
 		ss:   []string{"--glinet"},
-	}, {
-		name: "disable_mem_opt",
-		opts: options{disableMemoryOptimization: true},
-		ss:   []string{"--no-mem-optimization"},
 	}, {
 		name: "multiple",
 		opts: options{
-			serviceControlAction:      "run",
+			serviceControlAction: "run",
-			configFilename:            "config",
+			configFilename:       "config",
-			workDir:                   "work",
+			workDir:              "work",
-			pidFile:                   "pid",
+			pidFile:              "pid",
-			disableUpdate:             true,
+			disableUpdate:        true,
-			disableMemoryOptimization: true,
 		},
 		ss: []string{
 			"-c", "config",
@@ -193,18 +190,13 @@ func TestSerialize(t *testing.T) {
 			"-s", "run",
 			"--pidfile", "pid",
 			"--no-check-update",
-			"--no-mem-optimization",
 		},
 	}}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			result := serialize(tc.opts)
-			require.Lenf(t, result, len(tc.ss), reportFmt, tc.ss, result)
+			assert.ElementsMatch(t, tc.ss, result)
-
-			for i, r := range result {
-				assert.Equalf(t, tc.ss[i], r, reportFmt, tc.ss, result)
-			}
 		})
 	}
 }