Make directory operations methods on Directory - #11945
Conversation
dee18ce to
3695b7a
Compare
3695b7a to
7959b78
Compare
|
[approve ci autest 2] |
Some static functions can now take `Directory` parameters instead of `Stripe`.
7959b78 to
388d7ab
Compare
|
Rebased on master. |
|
I have not removed the dependency of |
| { | ||
| CACHE_TRY_LOCK(lock, stripe->mutex, mutex->thread_holding); | ||
| if (!lock.is_locked() || (od = stripe->open_read(key)) || dir_probe(key, stripe, &result, &last_collision)) { | ||
| if (!lock.is_locked() || (od = stripe->open_read(key)) || stripe->directory.probe(key, stripe, &result, &last_collision)) { |
There was a problem hiding this comment.
Note for later refactoring: this can be like this?
stripe->dir_probe(key, &result, &last_collision)
| // Note : abuse of the token bit in dir entries | ||
| int | ||
| dir_bucket_loop_fix(Dir *start_dir, int s, Stripe *stripe) | ||
| dir_bucket_loop_fix(Dir *start_dir, int s, Directory *directory) |
There was a problem hiding this comment.
Can we make this function a member of Directory too?
There was a problem hiding this comment.
Definitely, I missed this opportunity. Very easy improvement for further refactoring. Thanks for catching it.
There was a problem hiding this comment.
I think dir_init_segment can be a method of Directory as well.
|
|
||
| inline void | ||
| unlink_from_freelist(Dir *e, int s, Stripe *stripe) | ||
| unlink_from_freelist(Dir *e, int s, Directory *directory) |
There was a problem hiding this comment.
Similar to above. Would you share me what's the policy of which function should be moved to Directory or not here?
There was a problem hiding this comment.
I looked for the functions that were operations on the directory, and I might have missed this one.
There was a problem hiding this comment.
Yeah, this should be moved. I see what happened. I only moved the public interface, and not any helper functions. Further refactoring should make all these helper functions private methods on Directory.
masaori335
left a comment
masaori335
left a comment
There was a problem hiding this comment.
This just moves Dir operators into Directory for cleanup. Probably, we need more refactoring in this area, but this seems good to me.
4 participants
These operations are working with directory data. Right now, they operate on data of type
StripeorStripeSM, but that was because those data structures contained the directory data they needed until recently. Most of the other data there is irrelevant.This PR moves those operations (see commits for the list) into the
Directorystructure to reduce the dependence on the irrelevant parts ofStripeandStripeSM.The
StripeSMsare needed in some cases for debugging, metrics, and an intriguing invocation ofStripe::dir_validfromdir_probe; theDirectorystructure will need to be adjusted to have access to a few things for those purposes, but the details of that are still a work in progress.