Skip to content

Commit

Permalink
revert use of mmap-go (#706)
Browse files Browse the repository at this point in the history
We have a suspicion that when we switched to using the mmap-go library
it contributed to an issue where zoekt-webserver stops responding on
some linux versions. Our suspicion has something to do from us
hardcoding the page size to 4k to asking the kernel and how that
interacts with THP. Given we don't actually build for windows anymore,
we are partially reverting the mmap changes from
7424ac8

Test Plan: go test ./... on darwin. Then CI for linux testing.
  • Loading branch information
keegancsmith authored Nov 29, 2023
1 parent 8abf874 commit 0d03621
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 28 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ require (
github.com/andygrunwald/go-gerrit v0.0.0-20230628115649-c44fe2fbf2ca
github.com/bmatcuk/doublestar v1.3.4
github.com/dustin/go-humanize v1.0.1
github.com/edsrzf/mmap-go v1.1.0
github.com/felixge/fgprof v0.9.3
github.com/fsnotify/fsnotify v1.6.0
github.com/gfleury/go-bitbucket-v1 v0.0.0-20230626192437-8d7be5866751
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
github.com/edsrzf/mmap-go v1.1.0 h1:6EUwBLQ/Mcr1EYLE4Tn1VdW1A4ckqCQWZBw8Hr0kjpQ=
github.com/edsrzf/mmap-go v1.1.0/go.mod h1:19H/e8pUPLicwkyNgOykDXkJ9F0MHE+Z52B8EIth78Q=
github.com/elazarl/goproxy v0.0.0-20221015165544-a0805db90819 h1:RIB4cRk+lBqKK3Oy0r2gRX4ui7tuhiZq2SuTtTCi0/0=
github.com/elazarl/goproxy v0.0.0-20221015165544-a0805db90819/go.mod h1:Ro8st/ElPeALwNFlcTpWmkr6IoMFfkjXAvTHpevnDsM=
github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc=
Expand Down
62 changes: 62 additions & 0 deletions indexfile_other.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright 2016 Google Inc. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//go:build !linux && !darwin
// +build !linux,!darwin

package zoekt

import (
"fmt"
"os"
)

// NewIndexFile returns a new index file. The index file takes
// ownership of the passed in file, and may close it.
func NewIndexFile(f *os.File) (IndexFile, error) {
return &indexFileFromOS{f}, nil
}

type indexFileFromOS struct {
f *os.File
}

func (f *indexFileFromOS) Read(off, sz uint32) ([]byte, error) {
r := make([]byte, sz)
_, err := f.f.ReadAt(r, int64(off))
return r, err
}

func (f indexFileFromOS) Size() (uint32, error) {
fi, err := f.f.Stat()
if err != nil {
return 0, err
}

sz := fi.Size()

if sz >= maxUInt32 {
return 0, fmt.Errorf("overflow")
}

return uint32(sz), nil
}

func (f indexFileFromOS) Close() {
f.f.Close()
}

func (f indexFileFromOS) Name() string {
return f.f.Name()
}
35 changes: 10 additions & 25 deletions indexfile.go → indexfile_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,22 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//go:build linux || darwin

package zoekt

import (
"fmt"
"log"
"os"
"runtime"

// cross-platform memory-mapped file package.
// Benchmarks the same speed as syscall/unix Mmap
// see https://github.com/peterguy/benchmark-mmap
mmap "github.com/edsrzf/mmap-go"
"golang.org/x/sys/unix"
)

type mmapedIndexFile struct {
name string
size uint32
data mmap.MMap
data []byte
}

func (f *mmapedIndexFile) Read(off, sz uint32) ([]byte, error) {
Expand All @@ -48,25 +46,11 @@ func (f *mmapedIndexFile) Size() (uint32, error) {
}

func (f *mmapedIndexFile) Close() {
if err := f.data.Unmap(); err != nil {
log.Printf("WARN failed to memory unmap %s: %v", f.name, err)
if err := unix.Munmap(f.data); err != nil {
log.Printf("WARN failed to Munmap %s: %v", f.name, err)
}
}

func bufferSize(f *mmapedIndexFile) int {
// On Unix/Linux, mmap likes to allocate memory in
// page-sized chunks, so round up to the OS page size.
// mmap will zero-fill the extra bytes.
// On Windows, the Windows API CreateFileMapping method
// requires a buffer the same size as the file.
bsize := int(f.size)
if runtime.GOOS != "windows" {
pagesize := os.Getpagesize() - 1
bsize = (bsize + pagesize) &^ pagesize
}
return bsize
}

// NewIndexFile returns a new index file. The index file takes
// ownership of the passed in file, and may close it.
func NewIndexFile(f *os.File) (IndexFile, error) {
Expand All @@ -86,10 +70,11 @@ func NewIndexFile(f *os.File) (IndexFile, error) {
size: uint32(sz),
}

r.data, err = mmap.MapRegion(f, bufferSize(r), mmap.RDONLY, 0, 0)
rounded := (r.size + 4095) &^ 4095
r.data, err = unix.Mmap(int(f.Fd()), 0, int(rounded), unix.PROT_READ, unix.MAP_SHARED)
if err != nil {
return nil, fmt.Errorf("NewIndexFile: unable to memory map %s: %w", f.Name(), err)
return nil, err
}

return r, nil
return r, err
}

0 comments on commit 0d03621

Please sign in to comment.