Various refactoring #20
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We no longer have to impersonate macFUSE after cgofuse@84c0898ad2e02c4855e62a0dd45f4e163120b135.
djdv
force-pushed
the
j/ipfs-fs-extensions
branch
from
946c1aa to
7994749
Compare
Don't convert IPFS pins' CIDs
ci: remove special handling for FUSE-T
The map version was only for prototyping. It's sub-optimal and the implementation was bad since it's memory would grow indefinitely. The slice based approach should be much faster and memory efficient. While not measured, lock contention seems reduced as a result. Interactive use with a mounted system is perceptibly more responsive.
- packing struct - clarify UID and GID come from FUSE context - initial implementation for `rewinddir` - linting + lexical reordering - break functions up a little more
This made more sense in the old interface where Files and Directories were intentionally/strongly distinct. `fs.FS` intentionally conflates these constructs, which in turn obviates this interface. It would still be nice for implicit type checking at compile time. But it's not worth requiring implementations to implement the method. The Go `fs.FS` standard convention favors dynamic type assertions at the call site, with errors also being returned at runtime when the types do not match what's required/expected by the caller of the more general `FS.Open` method.
These values were not inheriting the type in the way they would have if `iota` was used. We can fix this and simplify the declaration by just using iota instead of explicitly defining values.
These don't matter yet, but lets not forget them. Later we should probably remove the logging, or place the locks before the print calls.
This should be temporary until 1.20 lands. All calls to `fserrors`.Join will revert to `errors.Join` when it's in the standard lib.
This is not proper, it's just housekeeping for now. The operations still need to be linted, with all the old code removed. And actual implemented correctly with regard to the new FS interface + FUSE standards.
Maybe it's better, maybe it's worse, maybe I broke it. Still a work in progress. But it seems faster. We need real tests and benchmarks would be nice, since this is the hottest code path for FUSE, and thus a valid optimization target.
This could probably still be better, but at least it's (seemingly) correct now.
I can't wait for this to cause conflicts and change again later anyway.
We still need to audit where permissions are being used. This just uses something closer to a Go style than the POSIX names. These names may change later, but are currently exported for translators to use. E.g. Go bits -> Fuse bits, et al.
Adds some consistency for these values across pkgs. And adds a translator to go from Go bits to FUSE bits. `getattr` now uses permissions from an `fs.FS` with a fallback if not.
Truthfully, I didn't benchmark this or look at the assembly. But I remember this pattern being a concern in the past. Unless the compiler has changed to optimize this out, things like the map literal would be initialized each call. Now we should do that only once on init. Metrics would be nice to see on this though. Does the compiler actually still act that way, and does the stack climbing cost us more than doing that anyway? Should these be lazy and initialized on the first call rather than at init time?
djdv
force-pushed
the
j/ipfs-fs-extensions
branch
from
caa06f5 to
bc43af4
Compare
This was referenced Nov 26, 2022
Merged
Merged
|
The work for this was broken into different phases which all eventually got merged into master. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to: #19
The current version of the code was/is pretty nasty as it was hastily ported from 1 (old custom
FileSystem) interface to a completely different one (new standardfs.FS).These changes should be slightly more proper.
This PR is likely going to be closed, and each of the commits in here will be separated out into smaller change sets.
Draft notes:
errors.Joinwhich isn't released yet. This will bump the Go version from 1.19 to 1.20.(CI is currently unhappy about this)
Depending on timing, we might make a constraint that shims this into 1.19.
Typically that will be HTTP over TCP between our program and the IPFS node.
My local setup typically uses Unix domain sockets where possible, which is about as fast as memcopy as we can get without being in the same process. And the same HTTP over TCP for LAN machines. The performance when testing these is fine which implies we're likely not introducing our own bottleneck.
(Go tests would be ideal, but I might not write those until the interfaces are less likely to change.)
fs.FSextension we added; which streams a directory via a channel.As-is we use the standard
ReadDirFilestyle within our FUSE'sreaddirimplementation.Reading 1 entry at a time, until we hit the end or FUSE tells us to stop.
This isn't optimal since Go forces us to wrap these entries slices, which in our case are always single elements.
This is unlikely to make a significant difference, but it might. We have no need to box+unbox them like that when we can just pull entries directly from a channel until we're canceled or done.
rewinddirnot implemented for FUSE yet.fs.FSinterface extensions don't seem necessary anymore. This is still in flux but as-is I reduced things and simplified some usage.