Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(filestore): add mmap reader option #665

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Dreamacro
Copy link
Contributor

Motivation

I have a custom IPFS implementation based on boxo, which uses FileManager on Windows because I need to store files directly rather than file blocks. The pull logic is roughly as follows:

  1. Pull the entire ProtoNode tree.
  2. Create a sparse file, which can be larger than 100 GB.
  3. Randomly pull RawNode file blocks and write them to the system. Meanwhile, this Windows node also communicates and synchronizes with other nodes.

When hundreds of Windows nodes are working simultaneously, I noticed that the system's memory consumption becomes very high, and the memory is immediately released once the file download is complete. Interestingly, the memory consumption is not by the program itself but by the operating system. After some investigation, I found the root cause: Windows caches the read file blocks (even if the *os.File is already closed).

This can be reproduced with the following code:

package main

import (
	"crypto/rand"
	"fmt"
	"io"
	"os"
	"runtime"
	"sync"
	"time"
	"unsafe"

	"sparse/ratelimit"

	"golang.org/x/sys/windows"
)

func readBuf(path string, idx int, size int) []byte {
	f, err := os.Open(path)
	if err != nil {
		panic(err)
	}
	defer f.Close()

	buf := make([]byte, size)
	if _, err := f.ReadAt(buf, int64(idx)); err != nil {
		panic(err)
	}

	return buf
}

type FILE_SET_SPARSE_BUFFER struct {
	SetSparse bool
}

// SetSparse makes the file be a sparse file
func SetSparse(out *os.File, open bool) error {
	lpInBuffer := FILE_SET_SPARSE_BUFFER{
		SetSparse: open,
	}

	var bytesReturned uint32
	err := windows.DeviceIoControl(
		windows.Handle(out.Fd()),
		windows.FSCTL_SET_SPARSE,
		(*byte)(unsafe.Pointer(&lpInBuffer)),
		uint32(unsafe.Sizeof(lpInBuffer)),
		nil, 0, &bytesReturned, nil,
	)
	if err != nil {
		return fmt.Errorf("DeviceIoControl FSCTL_SET_SPARSE: %w", err)
	}
	return nil
}

func main() {
	filename := "test.bin"
	size := 1024 * 1024 * 1024 * 100 // 100GB
	defer os.Remove(filename)

	f, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE, 0o644)
	if err != nil {
		panic(err)
	}
	defer f.Close()

	if err := SetSparse(f, true); err != nil {
		panic(err)
	}

	if err := f.Truncate(int64(size)); err != nil {
		panic(err)
	}

	chunkSize := 256 * 1024
	offset := make([]int, size/chunkSize)
	for i := 0; i < int(size); i += chunkSize {
		offset = append(offset, i)
	}

	mrand.Shuffle(len(offset), func(i, j int) {
		offset[i], offset[j] = offset[j], offset[i]
	})

	bucket := ratelimit.NewFromMbps(200)

	ch := make(chan int)

	cache := map[int]struct{}{}

	rwMux := sync.RWMutex{}

	numCpu := runtime.GOMAXPROCS(0)

	println(numCpu)

	rf, err := os.Open(filename)
	if err != nil {
		panic(err)
	}
	defer rf.Close()

	for i := 0; i < numCpu; i++ {
		go func() {
			for {
				rwMux.RLock()
				var key int
				for k := range cache {
					key = k
					break
				}
				rwMux.RUnlock()
				readBuf(filename, key, chunkSize)
				time.Sleep(time.Millisecond * 1)
			}
		}()
	}

	wg := sync.WaitGroup{}
	for i := 0; i < numCpu; i++ {
		wg.Add(1)
		go func() {
			defer wg.Done()
			buf := make([]byte, chunkSize)
			for idx := range ch {
				io.ReadFull(rand.Reader, buf)
				bucket.Wait(int64(chunkSize))

				if _, err := f.WriteAt(buf, int64(idx)); err != nil {
					panic(err)
				}

				// if err := f.Sync(); err != nil {
				// 	panic(err)
				// }

				rwMux.Lock()
				cache[idx] = struct{}{}
				rwMux.Unlock()
			}
		}()
	}

	for _, start := range offset {
		ch <- start
	}

	wg.Wait()
}

Solution

Using mmap can solve this problem (implemented with CreateFileMapping on Windows). To maintain backward compatibility, a WithMMapReader option has been added to FileManager. Enabling this option on Windows can prevent excessive memory consumption.

The downside is that it relies on the exp package, but it only changes from an indirect dependency to a direct dependency. Alternatively, the code from this package can be copied directly into the project.

@Dreamacro Dreamacro requested a review from a team as a code owner September 4, 2024 09:04
Copy link

codecov bot commented Sep 4, 2024

Codecov Report

Attention: Patch coverage is 83.78378% with 6 lines in your changes missing coverage. Please review.

Project coverage is 60.41%. Comparing base (29598b2) to head (06b91df).

Files with missing lines Patch % Lines
filestore/filereader.go 70.00% 4 Missing and 2 partials ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #665      +/-   ##
==========================================
+ Coverage   60.39%   60.41%   +0.02%     
==========================================
  Files         243      244       +1     
  Lines       31025    31054      +29     
==========================================
+ Hits        18737    18761      +24     
- Misses      10621    10629       +8     
+ Partials     1667     1664       -3     
Files with missing lines Coverage Δ
filestore/fsrefstore.go 41.90% <100.00%> (+4.09%) ⬆️
filestore/filereader.go 70.00% <70.00%> (ø)

... and 10 files with indirect coverage changes

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Dreamacro!

Triage notes:

@Dreamacro
Copy link
Contributor Author

@lidel Yes, mmap was introduced to solve the Windows-specific problem, but WithMMapReader is an option-in and the default behavior is still to use go std's os.File, so I implemented WithMMapReader to let the user choose whether to use mmap or not, I don't think a build tag is needed here, mmap is worked well to all major platforms.

@lidel lidel marked this pull request as draft September 17, 2024 16:31
@lidel lidel mentioned this pull request Sep 17, 2024
34 tasks
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Triage notes:

  • overall sounds good, as long it remains opt-in
    • we will dogfood this in Kubo, will circle back to this as part of 0.31 activities Release 0.31 kubo#10499
    • because mmap impl. is in golang.org/x/exp plan is to have it as opt-in flag for now, similar to IPFS_FD_MAX
  • small ask to rename func inline

filestore/fsrefstore.go Outdated Show resolved Hide resolved
@Dreamacro
Copy link
Contributor Author

done

@Dreamacro
Copy link
Contributor Author

@lidel Hi, should I do anything else?

@gammazero gammazero mentioned this pull request Oct 17, 2024
28 tasks
@Dreamacro Dreamacro marked this pull request as ready for review October 28, 2024 02:30
@lidel lidel self-assigned this Oct 29, 2024
Copy link
Contributor

@hsanjuan hsanjuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eyeballed this and it looks good to me. I defer to @lidel for merging.

@lidel lidel mentioned this pull request Nov 14, 2024
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants