]> Cypherpunks repositories - gostls13.git/commitdiff
mime: style, perf, and test updates to case-insensitive lookups
authorBrad Fitzpatrick <bradfitz@golang.org>
Thu, 28 Aug 2014 18:07:46 +0000 (11:07 -0700)
committerBrad Fitzpatrick <bradfitz@golang.org>
Thu, 28 Aug 2014 18:07:46 +0000 (11:07 -0700)
Only grab the lock once, don't allocate, add more tests.

LGTM=ruiu
R=ruiu, josharian
CC=golang-codereviews
https://golang.org/cl/139780043

src/pkg/mime/type.go
src/pkg/mime/type_test.go

index d1e403f0224c9b1ba1ce9a5b99eae810143c78d9..ffda1f0ce5fed6eeecde103fc14938a9536e0a42 100644 (file)
@@ -13,12 +13,7 @@ import (
 
 var (
        mimeLock       sync.RWMutex
-       mimeTypes      = map[string]string{}
-       mimeTypesLower = map[string]string{}
-)
-
-func init() {
-       mimeTypes := map[string]string{
+       mimeTypesLower = map[string]string{
                ".css":  "text/css; charset=utf-8",
                ".gif":  "image/gif",
                ".htm":  "text/html; charset=utf-8",
@@ -29,12 +24,21 @@ func init() {
                ".png":  "image/png",
                ".xml":  "text/xml; charset=utf-8",
        }
-       for ext, typ := range mimeTypes {
-               AddExtensionType(ext, typ)
+       mimeTypes = clone(mimeTypesLower)
+)
+
+func clone(m map[string]string) map[string]string {
+       m2 := make(map[string]string, len(m))
+       for k, v := range m {
+               m2[k] = v
+               if strings.ToLower(k) != k {
+                       panic("keys in mimeTypesLower must be lowercase")
+               }
        }
+       return m2
 }
 
-var once sync.Once
+var once sync.Once // guards initMime
 
 // TypeByExtension returns the MIME type associated with the file extension ext.
 // The extension ext should begin with a leading dot, as in ".html".
@@ -50,21 +54,41 @@ var once sync.Once
 //   /etc/apache2/mime.types
 //   /etc/apache/mime.types
 //
-// Windows system MIME types are extracted from registry.
+// On Windows, MIME types are extracted from the registry.
 //
 // Text types have the charset parameter set to "utf-8" by default.
 func TypeByExtension(ext string) string {
        once.Do(initMime)
        mimeLock.RLock()
-       typename := mimeTypes[ext]
-       mimeLock.RUnlock()
-       if typename == "" {
-               lower := strings.ToLower(ext)
-               mimeLock.RLock()
-               typename = mimeTypesLower[lower]
-               mimeLock.RUnlock()
+       defer mimeLock.RUnlock()
+
+       // Case-sensitive lookup.
+       v := mimeTypes[ext]
+       if v != "" {
+               return v
+       }
+
+       // Case-insensitive lookup.
+       // Optimistically assume a short ASCII extension and be
+       // allocation-free in that case.
+       var buf [10]byte
+       lower := buf[:0]
+       const utf8RuneSelf = 0x80 // from utf8 package, but not importing it.
+       for i := 0; i < len(ext); i++ {
+               c := ext[i]
+               if c >= utf8RuneSelf {
+                       // Slow path.
+                       return mimeTypesLower[strings.ToLower(ext)]
+               }
+               if 'A' <= c && c <= 'Z' {
+                       lower = append(lower, c+('a'-'A'))
+               } else {
+                       lower = append(lower, c)
+               }
        }
-       return typename
+       // The conversion from []byte to string doesn't allocate in
+       // a map lookup.
+       return mimeTypesLower[string(lower)]
 }
 
 // AddExtensionType sets the MIME type associated with
index 3ec86fbb54789a0dc74c6d9596863c5d0d8e0701..d2d254ae9a5d7dfa6b35aeef0d95c1c0ae8a2894 100644 (file)
@@ -19,17 +19,36 @@ func TestTypeByExtension(t *testing.T) {
        }
 }
 
-func TestCustomExtension(t *testing.T) {
-       custom := "test/test; charset=iso-8859-1"
-       if error := AddExtensionType(".tesT", custom); error != nil {
-               t.Fatalf("error %s for AddExtension(%s)", error, custom)
+func TestTypeByExtensionCase(t *testing.T) {
+       const custom = "test/test; charset=iso-8859-1"
+       const caps = "test/test; WAS=ALLCAPS"
+       if err := AddExtensionType(".TEST", caps); err != nil {
+               t.Fatalf("error %s for AddExtension(%s)", err, custom)
        }
-       // test with same capitalization
-       if registered := TypeByExtension(".tesT"); registered != custom {
-               t.Fatalf("registered %s instead of %s", registered, custom)
+       if err := AddExtensionType(".tesT", custom); err != nil {
+               t.Fatalf("error %s for AddExtension(%s)", err, custom)
        }
-       // test with different capitalization
-       if registered := TypeByExtension(".Test"); registered != custom {
-               t.Fatalf("registered %s instead of %s", registered, custom)
+
+       // case-sensitive lookup
+       if got := TypeByExtension(".tesT"); got != custom {
+               t.Fatalf("for .tesT, got %q; want %q", got, custom)
+       }
+       if got := TypeByExtension(".TEST"); got != caps {
+               t.Fatalf("for .TEST, got %q; want %s", got, caps)
+       }
+
+       // case-insensitive
+       if got := TypeByExtension(".TesT"); got != custom {
+               t.Fatalf("for .TesT, got %q; want %q", got, custom)
+       }
+}
+
+func TestLookupMallocs(t *testing.T) {
+       n := testing.AllocsPerRun(10000, func() {
+               TypeByExtension(".html")
+               TypeByExtension(".HtML")
+       })
+       if n > 0 {
+               t.Errorf("allocs = %v; want 0", n)
        }
 }