-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor parts of block_fs.cpp
#3233
Conversation
libres/lib/res_util/block_fs.cpp
Outdated
@@ -1096,22 +1074,18 @@ void block_fs_fwrite_buffer(block_fs_type *block_fs, const char *filename, | |||
|
|||
void block_fs_fread_realloc_buffer(block_fs_type *block_fs, | |||
const char *filename, buffer_type *buffer) { | |||
block_fs_aquire_rlock(block_fs); | |||
std::lock_guard guard{block_fs->mutex}; | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove extra block...?
libres/lib/res_util/block_fs.cpp
Outdated
free(data_ext); | ||
free(lock_ext); | ||
} | ||
|
||
/* | ||
This function is called both when allocating a new block_fs | ||
instance, and when an existing block_fs instance is 'rotated'. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"rotated" mentioned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look again 🪄
Note how the only place in production where the `block_level_lock` is set, it is fixed to `false`. The lock exists further up the stack inside `enkf_fs`. Therefore it is safe to remove this functionality. : w block_fs: remove unused `.lock` functionality Note how the only place in production where the `block_level_lock` is set, it is fixed to `false`. The lock exists further up the stack inside `enkf_fs`. Therefore it is safe to remove this functionality.
Note how this value is set only in one place, and it is set to `1.0`. The comment next to this line says that a fragmentation_limit of `1.0` means no fragmentation is run. Hence this is safe.
This is not necessary, as the paths are now only used for diagnostics and initialisation.
Codecov Report
@@ Coverage Diff @@
## main #3233 +/- ##
==========================================
- Coverage 65.68% 65.67% -0.02%
==========================================
Files 614 614
Lines 49045 48815 -230
Branches 4407 4405 -2
==========================================
- Hits 32216 32060 -156
+ Misses 15357 15271 -86
- Partials 1472 1484 +12
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup!
This is the commits from #3194 except for the controversial last one.