From f004d3ff4a95395f5ea3df9f26c8500bab3438e1 Mon Sep 17 00:00:00 2001
From: Aaron Jacobs <jacobsa@google.com>
Date: Wed, 22 Jul 2015 21:45:49 +1000
Subject: [PATCH] Avoid calling Logf on the hot path when logging is disabled.

The call to commonOp.Logf in sendBazilfuseResponse is extremely hot for
allocations, presumably because it needs to allocate an argument slice.
It is normally not needed at all, so why pay for it?
---
 connection.go          | 13 +++++++++++--
 fuseops/common_op.go   | 34 +++++++++++++++++++++++++---------
 fuseops/convert.go     |  2 ++
 mounted_file_system.go | 18 ++----------------
 4 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/connection.go b/connection.go
index ba9e168..5ee1922 100644
--- a/connection.go
+++ b/connection.go
@@ -50,6 +50,8 @@ type Connection struct {
 
 // Responsibility for closing the wrapped connection is transferred to the
 // result. You must call c.close() eventually.
+//
+// The loggers may be nil.
 func newConnection(
 	parentCtx context.Context,
 	debugLogger *log.Logger,
@@ -73,6 +75,10 @@ func (c *Connection) debugLog(
 	calldepth int,
 	format string,
 	v ...interface{}) {
+	if c.debugLogger == nil {
+		return
+	}
+
 	// Get file:line info.
 	var file string
 	var line int
@@ -239,8 +245,11 @@ func (c *Connection) ReadOp() (op fuseops.Op, err error) {
 		// Set up op dependencies.
 		opCtx := c.beginOp(bfReq)
 
-		debugLogForOp := func(calldepth int, format string, v ...interface{}) {
-			c.debugLog(opID, calldepth+1, format, v...)
+		var debugLogForOp func(int, string, ...interface{})
+		if c.debugLogger != nil {
+			debugLogForOp = func(calldepth int, format string, v ...interface{}) {
+				c.debugLog(opID, calldepth+1, format, v...)
+			}
 		}
 
 		finished := func(err error) { c.finishOp(bfReq) }
diff --git a/fuseops/common_op.go b/fuseops/common_op.go
index 2ae773b..f82580b 100644
--- a/fuseops/common_op.go
+++ b/fuseops/common_op.go
@@ -49,9 +49,13 @@ type commonOp struct {
 
 	// A function that can be used to log debug information about the op. The
 	// first argument is a call depth.
+	//
+	// May be nil.
 	debugLog func(int, string, ...interface{})
 
 	// A logger to be used for logging exceptional errors.
+	//
+	// May be nil.
 	errorLogger *log.Logger
 
 	// A function that is invoked with the error given to Respond, for use in
@@ -116,6 +120,10 @@ func (o *commonOp) Context() context.Context {
 }
 
 func (o *commonOp) Logf(format string, v ...interface{}) {
+	if o.debugLog == nil {
+		return
+	}
+
 	const calldepth = 2
 	o.debugLog(calldepth, format, v...)
 }
@@ -131,15 +139,19 @@ func (o *commonOp) Respond(err error) {
 	}
 
 	// Log the error.
-	o.Logf(
-		"-> (%s) error: %v",
-		o.op.ShortDesc(),
-		err)
+	if o.debugLog != nil {
+		o.Logf(
+			"-> (%s) error: %v",
+			o.op.ShortDesc(),
+			err)
+	}
 
-	o.errorLogger.Printf(
-		"(%s) error: %v",
-		o.op.ShortDesc(),
-		err)
+	if o.errorLogger != nil {
+		o.errorLogger.Printf(
+			"(%s) error: %v",
+			o.op.ShortDesc(),
+			err)
+	}
 
 	// Send a response to the kernel.
 	o.bazilReq.RespondError(err)
@@ -157,11 +169,15 @@ func (o *commonOp) sendBazilfuseResponse(resp interface{}) {
 	// Special case: handle successful ops with no response struct.
 	if resp == nil {
 		o.Logf("-> (%s) OK", o.op.ShortDesc())
+
 		respond.Call([]reflect.Value{})
 		return
 	}
 
 	// Otherwise, send the response struct to the kernel.
-	o.Logf("-> %v", resp)
+	if o.debugLog != nil {
+		o.Logf("-> %v", resp)
+	}
+
 	respond.Call([]reflect.Value{reflect.ValueOf(resp)})
 }
diff --git a/fuseops/convert.go b/fuseops/convert.go
index ab280e2..027b4b9 100644
--- a/fuseops/convert.go
+++ b/fuseops/convert.go
@@ -32,6 +32,8 @@ import (
 //
 // It is guaranteed that o != nil. If the op is unknown, a special unexported
 // type will be used.
+//
+// The debug logging function and error logger may be nil.
 func Convert(
 	opCtx context.Context,
 	r bazilfuse.Request,
diff --git a/mounted_file_system.go b/mounted_file_system.go
index 1778638..580ea6b 100644
--- a/mounted_file_system.go
+++ b/mounted_file_system.go
@@ -16,7 +16,6 @@ package fuse
 
 import (
 	"fmt"
-	"io/ioutil"
 	"log"
 	"runtime"
 
@@ -193,12 +192,6 @@ func Mount(
 	dir string,
 	server Server,
 	config *MountConfig) (mfs *MountedFileSystem, err error) {
-	// Arrange for a non-nil debug logger.
-	debugLogger := config.DebugLogger
-	if debugLogger == nil {
-		debugLogger = log.New(ioutil.Discard, "", 0)
-	}
-
 	// Initialize the struct.
 	mfs = &MountedFileSystem{
 		dir:                 dir,
@@ -206,7 +199,6 @@ func Mount(
 	}
 
 	// Open a bazilfuse connection.
-	debugLogger.Println("Opening a bazilfuse connection.")
 	bfConn, err := bazilfuse.Mount(mfs.dir, config.bazilfuseOptions()...)
 	if err != nil {
 		err = fmt.Errorf("bazilfuse.Mount: %v", err)
@@ -219,17 +211,11 @@ func Mount(
 		opContext = context.Background()
 	}
 
-	// Create a /dev/null error logger if necessary.
-	errorLogger := config.ErrorLogger
-	if errorLogger == nil {
-		errorLogger = log.New(ioutil.Discard, "", 0)
-	}
-
 	// Create our own Connection object wrapping it.
 	connection, err := newConnection(
 		opContext,
-		debugLogger,
-		errorLogger,
+		config.DebugLogger,
+		config.ErrorLogger,
 		bfConn)
 
 	if err != nil {