From 4cf6490e82edd288b91d6a786d85dab042015e24 Mon Sep 17 00:00:00 2001
From: Gregory Oschwald <goschwald@maxmind.com>
Date: Fri, 30 Dec 2016 16:38:52 -0800
Subject: [PATCH] Take advantage of string([]byte) optimization

Previously, we used unsafe to avoid copying the bytes from the buffer
when doing the map lookup for the struct field. The Go compiler has a
special optimization for string([]byte) lookups that we can take
advantage of instead:

https://github.com/golang/go/issues/3512

This reduces code complexity a bit and eliminates unsafe usage for
non-Windows builds.
---
 decoder.go       | 31 +++++++++++++++++++------------
 key_appengine.go |  7 -------
 key_other.go     | 28 ----------------------------
 3 files changed, 19 insertions(+), 47 deletions(-)
 delete mode 100644 key_appengine.go
 delete mode 100644 key_other.go

diff --git a/decoder.go b/decoder.go
index 53ab390..8d59b1b 100644
--- a/decoder.go
+++ b/decoder.go
@@ -416,9 +416,9 @@ func (d *decoder) decodeMap(size uint, offset uint, result reflect.Value) (uint,
 	}
 
 	for i := uint(0); i < size; i++ {
-		var key string
+		var key []byte
 		var err error
-		key, offset, err = d.decodeKeyString(offset)
+		key, offset, err = d.decodeKey(offset)
 
 		if err != nil {
 			return 0, err
@@ -429,7 +429,7 @@ func (d *decoder) decodeMap(size uint, offset uint, result reflect.Value) (uint,
 		if err != nil {
 			return 0, err
 		}
-		result.SetMapIndex(reflect.ValueOf(key), value.Elem())
+		result.SetMapIndex(reflect.ValueOf(string(key)), value.Elem())
 	}
 	return offset, nil
 }
@@ -534,13 +534,15 @@ func (d *decoder) decodeStruct(size uint, offset uint, result reflect.Value) (ui
 	for i := uint(0); i < size; i++ {
 		var (
 			err error
-			key string
+			key []byte
 		)
-		key, offset, err = d.decodeStructKey(offset)
+		key, offset, err = d.decodeKey(offset)
 		if err != nil {
 			return 0, err
 		}
-		j, ok := fields.namedFields[key]
+		// The string() does not create a copy due to this compiler
+		// optimization: https://github.com/golang/go/issues/3512
+		j, ok := fields.namedFields[string(key)]
 		if !ok {
 			offset = d.nextValueOffset(offset, 1)
 			continue
@@ -577,17 +579,22 @@ func uintFromBytes(prefix uint64, uintBytes []byte) uint64 {
 	return val
 }
 
-func (d *decoder) decodeKeyString(offset uint) (string, uint, error) {
-	typeNum, size, newOffset := d.decodeCtrlData(offset)
+// decodeKey decodes a map key into []byte slice. We use a []byte so that we
+// can take advantage of https://github.com/golang/go/issues/3512 to avoid
+// copying the bytes when decoding a struct. Previously, we achieved this by
+// using unsafe.
+func (d *decoder) decodeKey(offset uint) ([]byte, uint, error) {
+	typeNum, size, dataOffset := d.decodeCtrlData(offset)
 	if typeNum == _Pointer {
-		pointer, ptrOffset := d.decodePointer(size, newOffset)
-		key, _, err := d.decodeKeyString(pointer)
+		pointer, ptrOffset := d.decodePointer(size, dataOffset)
+		key, _, err := d.decodeKey(pointer)
 		return key, ptrOffset, err
 	}
 	if typeNum != _String {
-		return "", 0, newInvalidDatabaseError("unexpected type when decoding string: %v", typeNum)
+		return nil, 0, newInvalidDatabaseError("unexpected type when decoding string: %v", typeNum)
 	}
-	return d.decodeString(size, newOffset)
+	newOffset := dataOffset + size
+	return d.buffer[dataOffset:newOffset], newOffset, nil
 }
 
 // This function is used to skip ahead to the next value without decoding
diff --git a/key_appengine.go b/key_appengine.go
deleted file mode 100644
index c7dc6ef..0000000
--- a/key_appengine.go
+++ /dev/null
@@ -1,7 +0,0 @@
-// +build appengine
-
-package maxminddb
-
-func (d *decoder) decodeStructKey(offset uint) (string, uint, error) {
-	return d.decodeKeyString(offset)
-}
diff --git a/key_other.go b/key_other.go
deleted file mode 100644
index 577e6f8..0000000
--- a/key_other.go
+++ /dev/null
@@ -1,28 +0,0 @@
-// +build !appengine
-
-package maxminddb
-
-import (
-	"reflect"
-	"unsafe"
-)
-
-// decodeStructKey returns a string which points into the database. Don't keep
-// it around.
-func (d *decoder) decodeStructKey(offset uint) (string, uint, error) {
-	typeNum, size, newOffset := d.decodeCtrlData(offset)
-	switch typeNum {
-	case _Pointer:
-		pointer, ptrOffset := d.decodePointer(size, newOffset)
-		s, _, err := d.decodeStructKey(pointer)
-		return s, ptrOffset, err
-	case _String:
-		var s string
-		val := (*reflect.StringHeader)(unsafe.Pointer(&s))
-		val.Data = uintptr(unsafe.Pointer(&d.buffer[newOffset]))
-		val.Len = int(size)
-		return s, newOffset + size, nil
-	default:
-		return "", 0, newInvalidDatabaseError("unexpected type when decoding struct key: %v", typeNum)
-	}
-}