Skip to content

Commit

Permalink
Speed up fat32 by replacing clusters map with a slice
Browse files Browse the repository at this point in the history
  • Loading branch information
Frostman committed Sep 10, 2024
1 parent 0fddede commit 358c497
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 36 deletions.
2 changes: 1 addition & 1 deletion filesystem/fat32/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ func testReadFilesystemData() (info *testFSInfo, err error) {
rootDirCluster: 2, // root is at cluster 2
size: sizeInBytes,
maxCluster: numClusters,
clusters: make(map[uint32]uint32),
clusters: make([]uint32, numClusters+1),
}
case inClusters && len(clusterLineMatch) > 4:
start, err := strconv.Atoi(clusterLineMatch[1])
Expand Down
41 changes: 26 additions & 15 deletions filesystem/fat32/fat32.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,17 +228,16 @@ func Create(f util.File, size, start, blocksize int64, volumeLabel string) (*Fil
fatSecondaryStart := uint64(fatPrimaryStart) + uint64(fatSize)
maxCluster := fatSize / 4
rootDirCluster := uint32(2)
clusters := make([]uint32, maxCluster+1)
clusters[rootDirCluster] = eocMarker
fat := table{
fatID: fatID,
eocMarker: eocMarker,
unusedMarker: unusedMarker,
size: fatSize,
rootDirCluster: rootDirCluster,
clusters: map[uint32]uint32{
// when we start, there is just one directory with a single cluster
rootDirCluster: eocMarker,
},
maxCluster: maxCluster,
clusters: clusters,
maxCluster: maxCluster,
}

// where does our data start?
Expand Down Expand Up @@ -694,10 +693,9 @@ func (fs *FileSystem) getClusterList(firstCluster uint32) ([]uint32, error) {
// first, get the chain of clusters
complete := false
cluster := firstCluster
clusters := fs.table.clusters

// do we even have a valid cluster?
if _, ok := clusters[cluster]; !ok {
if cluster > fs.table.maxCluster || fs.table.clusters[cluster] == 0 {
return nil, fmt.Errorf("invalid start cluster: %d", cluster)
}

Expand All @@ -706,11 +704,13 @@ func (fs *FileSystem) getClusterList(firstCluster uint32) ([]uint32, error) {
// save the current cluster
clusterList = append(clusterList, cluster)
// get the next cluster
newCluster := clusters[cluster]
newCluster := fs.table.clusters[cluster]
// if it is EOC, we are done
switch {
case fs.table.isEoc(newCluster):
complete = true
case newCluster > fs.table.maxCluster:
return nil, fmt.Errorf("invalid cluster chain at %d", newCluster)
case cluster < 2:
return nil, fmt.Errorf("invalid cluster chain at %d", cluster)
}
Expand Down Expand Up @@ -924,6 +924,10 @@ func (fs *FileSystem) readDirWithMkdir(p string, doMake bool) (*Directory, []*di
// returns the indexes of clusters to be used in order. If the new size is smaller than
// the original size, will shrink the chain.
func (fs *FileSystem) allocateSpace(size uint64, previous uint32) ([]uint32, error) {
if previous > fs.table.maxCluster {
return nil, fmt.Errorf("invalid cluster chain at %d", previous)
}

var (
clusters []uint32
err error
Expand Down Expand Up @@ -961,12 +965,11 @@ func (fs *FileSystem) allocateSpace(size uint64, previous uint32) ([]uint32, err
}

// get a list of allocated clusters, so we can know which ones are unallocated and therefore allocatable
allClusters := fs.table.clusters
maxCluster := fs.table.maxCluster

if extraClusterCount > 0 {
for i := uint32(2); i < maxCluster && len(allocated) < extraClusterCount; i++ {
if _, ok := allClusters[i]; !ok {
if fs.table.clusters[i] == 0 {
// these become the same at this point
allocated = append(allocated, i)
}
Expand All @@ -982,12 +985,12 @@ func (fs *FileSystem) allocateSpace(size uint64, previous uint32) ([]uint32, err

// extend the chain and fill them in
if previous > 0 {
allClusters[previous] = allocated[0]
fs.table.clusters[previous] = allocated[0]
}
for i := 0; i < lastAlloc; i++ {
allClusters[allocated[i]] = allocated[i+1]
fs.table.clusters[allocated[i]] = allocated[i+1]
}
allClusters[allocated[lastAlloc]] = fs.table.eocMarker
fs.table.clusters[allocated[lastAlloc]] = fs.table.eocMarker

// update the FSIS
lastAllocatedCluster = allocated[len(allocated)-1]
Expand All @@ -1003,13 +1006,21 @@ func (fs *FileSystem) allocateSpace(size uint64, previous uint32) ([]uint32, err
}
deallocated = clusters[lastAlloc+1:]

if uint32(lastAlloc) > fs.table.maxCluster || clusters[lastAlloc] > fs.table.maxCluster { //nolint:gosec // lastAlloc is >= 0

Check failure on line 1009 in filesystem/fat32/fat32.go

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

directive `//nolint:gosec // lastAlloc is >= 0` is unused for linter "gosec" (nolintlint)

Check failure on line 1009 in filesystem/fat32/fat32.go

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

directive `//nolint:gosec // lastAlloc is >= 0` is unused for linter "gosec" (nolintlint)
return nil, fmt.Errorf("invalid cluster chain at %d", lastAlloc)
}

// mark last allocated one as EOC
allClusters[clusters[lastAlloc]] = fs.table.eocMarker
fs.table.clusters[clusters[lastAlloc]] = fs.table.eocMarker

// unmark all of the unused ones
lastAllocatedCluster = fs.fsis.lastAllocatedCluster
for _, cl := range deallocated {
allClusters[cl] = fs.table.unusedMarker
if cl > fs.table.maxCluster {
return nil, fmt.Errorf("invalid cluster chain at %d", cl)
}

fs.table.clusters[cl] = fs.table.unusedMarker
if cl == lastAllocatedCluster {
lastAllocatedCluster--
}
Expand Down
26 changes: 21 additions & 5 deletions filesystem/fat32/fat32_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ import (
in that case, the dataStart is relative to partition, not to disk, so need to read the offset correctly
*/

func clustersFromMap(m map[uint32]uint32, maxCluster uint32) []uint32 {
clusters := make([]uint32, maxCluster+1)
for k, v := range m {
clusters[k] = v
}

return clusters
}

func getValidFat32FSFull() *FileSystem {
fs := getValidFat32FSSmall()
fs.table = *getValidFat32Table()
Expand All @@ -25,11 +34,12 @@ func getValidFat32FSFull() *FileSystem {

func getValidFat32FSSmall() *FileSystem {
eoc := uint32(0xffffffff)
maxCluster := uint32(128)
fs := &FileSystem{
table: table{
rootDirCluster: 2,
size: 512,
maxCluster: 128,
maxCluster: maxCluster,
eocMarker: eoc,
/*
map:
Expand All @@ -40,8 +50,9 @@ func getValidFat32FSSmall() *FileSystem {
11
15
16-broken
17-broken
*/
clusters: map[uint32]uint32{
clusters: clustersFromMap(map[uint32]uint32{
2: eoc,
3: 4,
4: 5,
Expand All @@ -53,8 +64,9 @@ func getValidFat32FSSmall() *FileSystem {
9: 11,
11: eoc,
15: eoc,
16: 0,
},
16: 1,
17: 999,
}, maxCluster),
},
bytesPerCluster: 512,
dataStart: 178176,
Expand All @@ -80,6 +92,7 @@ func getValidFat32FSSmall() *FileSystem {
}
return fs
}

func TestFat32GetClusterList(t *testing.T) {
fs := getValidFat32FSSmall()

Expand Down Expand Up @@ -178,6 +191,7 @@ func TestFat32AllocateSpace(t *testing.T) {
11
15
16-broken
17-broken
// recall that 512 bytes per cluster here
*/
tests := []struct {
Expand All @@ -189,9 +203,11 @@ func TestFat32AllocateSpace(t *testing.T) {
{500, 2, []uint32{2}, nil},
{600, 2, []uint32{2, 12}, nil},
{2000, 2, []uint32{2, 12, 13, 14}, nil},
{2000, 0, []uint32{12, 13, 14, 17}, nil},
{2000, 0, []uint32{12, 13, 14, 18}, nil},
{200000000000, 0, nil, fmt.Errorf("no space left on device")},
{200000000000, 2, nil, fmt.Errorf("no space left on device")},
{2000, 17, nil, fmt.Errorf("unable to get cluster list: invalid cluster chain at 999")},
{2000, 999, nil, fmt.Errorf("invalid cluster chain at 999")},
}
for _, tt := range tests {
// reset for each test
Expand Down
19 changes: 9 additions & 10 deletions filesystem/fat32/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ package fat32

import (
"encoding/binary"
"reflect"
"slices"
)

// table a FAT32 table
type table struct {
fatID uint32
eocMarker uint32
unusedMarker uint32
clusters map[uint32]uint32
clusters []uint32
rootDirCluster uint32
size uint32
maxCluster uint32
Expand All @@ -28,7 +28,7 @@ func (t *table) equal(a *table) bool {
t.rootDirCluster == a.rootDirCluster &&
t.size == a.size &&
t.maxCluster == a.maxCluster &&
reflect.DeepEqual(t.clusters, a.clusters)
slices.Equal(a.clusters, t.clusters)
}

/*
Expand All @@ -37,12 +37,14 @@ func (t *table) equal(a *table) bool {
*/

func tableFromBytes(b []byte) *table {
maxCluster := uint32(len(b) / 4) //nolint:gosec // b isn't big enough to cause an overflow

Check failure on line 40 in filesystem/fat32/table.go

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

directive `//nolint:gosec // b isn't big enough to cause an overflow` is unused for linter "gosec" (nolintlint)

Check failure on line 40 in filesystem/fat32/table.go

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

directive `//nolint:gosec // b isn't big enough to cause an overflow` is unused for linter "gosec" (nolintlint)

t := table{
fatID: binary.LittleEndian.Uint32(b[0:4]),
eocMarker: binary.LittleEndian.Uint32(b[4:8]),
size: uint32(len(b)),
clusters: map[uint32]uint32{},
maxCluster: uint32(len(b) / 4),
size: uint32(len(b)), //nolint:gosec // b isn't big enough to cause an overflow

Check failure on line 45 in filesystem/fat32/table.go

View workflow job for this annotation

GitHub Actions / Test (macos-latest)

directive `//nolint:gosec // b isn't big enough to cause an overflow` is unused for linter "gosec" (nolintlint)

Check failure on line 45 in filesystem/fat32/table.go

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

directive `//nolint:gosec // b isn't big enough to cause an overflow` is unused for linter "gosec" (nolintlint)
clusters: make([]uint32, maxCluster+1),
maxCluster: maxCluster,
rootDirCluster: 2, // always 2 for FAT32
}
// just need to map the clusters in
Expand Down Expand Up @@ -71,10 +73,7 @@ func (t *table) bytes() []byte {
for i := uint32(2); i < numClusters; i++ {
bStart := i * 4
bEnd := bStart + 4
val := uint32(0)
if cluster, ok := t.clusters[i]; ok {
val = cluster
}
val := t.clusters[i]
binary.LittleEndian.PutUint32(b[bStart:bEnd], val)
}

Expand Down
8 changes: 3 additions & 5 deletions filesystem/fat32/table_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package fat32
import (
"bytes"
"os"
"slices"
"testing"

"github.com/diskfs/go-diskfs/util"
Expand All @@ -16,13 +17,10 @@ const (

func getValidFat32Table() *table {
// make a duplicate, in case someone modifies what we return
var t = &table{}
t := &table{}
*t = *fsInfo.table
// and because the clusters are copied by reference
t.clusters = make(map[uint32]uint32)
for k, v := range fsInfo.table.clusters {
t.clusters[k] = v
}
t.clusters = slices.Clone(t.clusters)

return t
}
Expand Down

0 comments on commit 358c497

Please sign in to comment.