Art integration#49
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ert, merged conflicts from angelo
…ndependent and other small changes
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR replaces the directory-entry backing store in the FUSE filesystem from a Go map to an Adaptive Radix Tree (ART), while updating filesystem code to use RWMutex for improved concurrent read behavior.
Changes:
- Replace
Dir.Nodes(map) withDir.tree(ART) and update filesystem operations/tests accordingly - Add a new
internal/artpackage implementing Insert/Search/Delete/Traverse and node growth/shrink - Adjust locking in
File/Dirto usesync.RWMutexand add an ART stress-test command
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/fs/fs.go | Initializes the root directory with an ART and inserts hello.txt; switches to RWMutex |
| internal/fs/dir.go | Reworks Lookup/ReadDirAll/Create/Mkdir/Remove/Rename to use the ART API |
| internal/fs/file.go | Switches Attr to RLock and fixes slice growth allocations to use int |
| internal/fs/fs_test.go | Updates tests to validate entries via ART search instead of map access |
| internal/art/*.go | Adds ART implementation (nodes, insert/search/delete, traversal, debug printing) |
| cmd/radFS/arttest/main.go | Adds a manual stress-test program for ART grow/shrink correctness |
| docs/weekly/angelo/week_4.md | Adds a weekly progress note related to ART implementation |
Comments suppressed due to low confidence (2)
internal/fs/dir.go:1
Mkdirno longer updates the parent directory’smtime/ctimeafter adding the new entry (the prior map-based version did). This is observable metadata behavior change for FUSE clients. After inserting into the tree, updated.mtime/d.ctimesimilarly toRemove.
package fs
internal/fs/dir.go:1
Createalso no longer updates the parent directory’smtime/ctimeafter creating/inserting the new file entry, which is a metadata regression from the previous implementation. Updated.mtime/d.ctimeafter the insert.
package fs
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "time" | ||
|
|
||
| "bazil.org/fuse/fs" | ||
| "github.com/acmpesuecc/radFS/internal/art" |
| uid: uint32(os.Getuid()), //permissions implemnet based on userid | ||
| gid: uint32(os.Getgid()), //permissions implement based on groupid |
| atime: time.Now(), | ||
| mtime: time.Now(), | ||
| ctime: time.Now(), | ||
| uid: uint32(os.Getuid()), //permissions implemnet based on userid |
| d.mu.RLock() | ||
| defer d.mu.RUnlock() | ||
|
|
||
| node, ok := d.Nodes[name] | ||
| v, ok := d.tree.Search([]byte(name)) |
| d.atime = time.Now() | ||
|
|
||
| return node, nil | ||
| return v.(fs.Node), nil |
| newData := make([]byte, int(req.Size)) | ||
| copy(newData, f.data) | ||
| f.data = newData |
| func (t *Tree) Search(key []byte) (interface{}, bool) { | ||
| leaf := search(t.root, key, 0) // start from root and depth 0 | ||
| if leaf != nil && isleaf(leaf) { | ||
| return leaf.leaf.values, true //Node->innerleaf->values | ||
| } | ||
| return "", false | ||
| } |
| if isleaf(n) { | ||
| if string(n.leaf.key) == string(key) { | ||
| return n | ||
| } | ||
| return nil | ||
| } |
| if isleaf(n) { | ||
| if string(n.leaf.key) == string(key) { | ||
| return nil, true | ||
| } | ||
| return n, false | ||
| } |
| func (t *Tree) Insert(key []byte, value interface{}) { | ||
| t.root = insert(t.root, value, key, 0) | ||
| } | ||
|
|
||
| func (t *Tree) Search(key []byte) (interface{}, bool) { | ||
| leaf := search(t.root, key, 0) // start from root and depth 0 | ||
| if leaf != nil && isleaf(leaf) { | ||
| return leaf.leaf.values, true //Node->innerleaf->values | ||
| } | ||
| return "", false | ||
| } | ||
| func (t *Tree) Delete(key []byte) bool { | ||
| if t.root == nil { | ||
| return false | ||
| } | ||
|
|
||
| newRoot, deleted := deletekey(t.root, key, 0) | ||
|
|
||
| if deleted { | ||
| t.root = newRoot | ||
| return true | ||
| } | ||
|
|
||
| return false | ||
| } |
| if d == newParent { | ||
| d.mu.Lock() | ||
| defer d.mu.Unlock() | ||
| } else { |
There was a problem hiding this comment.
Can deadlock when 2 concurrent processes using same 2 directories. For example: mv d1/f1 -> d2 and mv d2/f2 -> d1
i.e. an ABBA deadlock
| d.mu.Lock() | ||
| defer d.mu.Unlock() | ||
| d.mu.RLock() | ||
| defer d.mu.RUnlock() |
There was a problem hiding this comment.
Directory level lock => No point of ART
Implement art.Node-wise locking instead
internal/art/search.go, internal/art/insert.go and internal/art/delete.go have to be rewritten to be iterative instead if implementing this
|
|
||
| } | ||
|
|
||
| for i := 0; i < len(in.children); i++ { // find the free child |
There was a problem hiding this comment.
This is O(N) but should be O(1)
Maintain a bitmask with 1 being free and 0 being occupied and the position of the bit indicating the free child position.
Check for the first free index using math/bits.TrailingZeroes()
| defer d.mu.RUnlock() | ||
|
|
||
| node, ok := d.Nodes[name] | ||
| v, ok := d.tree.Search([]byte(name)) |
There was a problem hiding this comment.
Avoid casting string to []byte. Use the string directly
| return n | ||
| } | ||
|
|
||
| func copymeta(n *Node, new_node *Node) { |
There was a problem hiding this comment.
This can be a very frequently called function when doing operations so cap the internal/art/node.go meta struct at 8 bytes as described in the ART paper Section III.E and just copy by assignment
|
|
||
| } | ||
|
|
||
| func deepcopy(source []byte) []byte { |
There was a problem hiding this comment.
Not needed if internal/art/util.go line 304 is resolved
ART INTEGRATION
Context
integrating the ART data stricture with our file system
Description
Replace the hash map data structure used as a place holder and replace it with art and its crud operations
Changes
And apply operations through a callback function.
Tests
fs_test.go ran again; test suite works
Docs