Relaxed lookahead alignment, other internal block alloc readability improvements #912
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
- Renamed lfs.free -> lfs.lookahead - Renamed lfs.free.off -> lfs.lookahead.start - Renamed lfs.free.i -> lfs.lookahead.next - Renamed lfs.free.ack -> lfs.lookahead.ckpoint - Renamed lfs_alloc_ack -> lfs_alloc_ckpoint These have been named a bit confusingly, and I think the new names make their relevant purposes a bit clearer. At the very it's clear lfs.lookahead is related to the lookahead buffer. (and doesn't imply a closed free-bitmap).
Some of this is just better documentation, some of this is reworking the logic to be more intention driven... if that makes sense...
This drops the lookahead buffer from operating on 32-bit words to operating on 8-bit bytes, and removes any alignment requirement. This may have some minor performance impact, but it is unlikely to be significant when you consider IO overhead. The original motivation for 32-bit alignment was an attempt at future-proofing in case we wanted some more complex on-disk data structure. This never happened, and even if it did, it could have been added via additional config options. This has been a significant pain point for users, since providing word-aligned byte-sized buffers in C can be a bit annoying.
geky
added
enhancement
needs minor version
Tests passed ✓, Code: 16800 B (-0.1%), Stack: 1432 B (-1.1%), Structs: 800 B (+0.0%)
|
|
Hi, could you please also update comment in Especially it still mentions 64-bit alignment for malloc requirement: Line 218 in 3513ff1 |
The only reason we needed this alignment was for the lookahead buffer. Now that the lookahead buffer is relaxed to operate on bytes, we can relax our malloc alignment requirement all the way down to the byte level, since we mainly use lfs_malloc to allocate byte-level buffers. This does introduce a risk that we might need word-level mallocs in the future. If that happens we will need to decide if changing the malloc alignment is a breaking change, or gate alignment requirements behind user provided defines. Found by HiFiPhile.
|
Good catch, should be updated now. There is a risk that we will need word-alignment in the future, but I guess we'll address that when we get to that. |
Tests passed ✓, Code: 16808 B (-0.1%), Stack: 1432 B (-1.1%), Structs: 800 B (+0.0%)
|
geky
added
next minor
and removed
needs minor version
Merged
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.
This drops the lookahead buffer from operating on 32-bit words to operating on 8-bit bytes, and removes any alignment requirement. This may have some minor performance impact, but it is unlikely to be significant when you consider IO overhead.
The original motivation for 32-bit alignment was an attempt at future-proofing in case we wanted some more complex on-disk data structure. This never happened, and even if it did, it could have been added via additional config options.
This has been a significant pain point for users, as providing word-aligned byte-sized buffers in C can be a bit annoying.
This also includes a number of internal block allocator improvements, mostly around naming things and making the logic hopefully easier to understand.