Removed commit penalty from shrub eviction heuristic

Old heuristic: estimate + commit > shrub_size/2
New heuristic: estimate > shrub_size/2 || estimate + commit > shrub_size

The goal here is two fold:

1. Prevent shrubs from exceeding shrub_size

2. Avoid runaway performance issues with repeatedly recalculating the
   exact estimate as a shrub approaches shrub_size

The 1/2 factor helps with 2., by evicting early, much like how our rbyds
determine when to split.

Since pending commits aren't included in the exact estimate, previously
we just added the pending commit estimate to the exact estimate before
checking our shrub heuristic. This gave us a nice single heuristic.

But it's important to note our shrubs are actually quite small. And our
commit estimate is really quite conservative. So there's real risk of
penalizing our shrubs to the point where they're difficult to leverage
on real geometry. At this scale, the extra ~40 B per commit attr
assuming uncompressed leb128s has a real impact.

If we consider that our shrub eviction heuristic is simulating a small
rbyd, it's interesting to note that rbyds are not penalized for pending
commits. Pending commits are simply required to always fit in
block_size/2. Doesn't fit? Error.

We don't quite have that freedom in the shrubs, but we can avoid
penalizing shrubs for commits, as long as we also check that the
estimate + commit does not exceed the hard shrub_size limit.

This heuristic probably deserves more scrutiny in the future, but this
at least seemed like a reasonable optimization to make.
This commit is contained in:
Christopher Haster
2024-01-09 21:59:15 -06:00
parent 5d0b116935
commit da726c1376
+21 -17
View File
@@ -9528,40 +9528,44 @@ static int lfsr_file_commit(lfs_t *lfs, lfsr_file_t *file,
// figure out how much data this commit progs // figure out how much data this commit progs
lfs_size_t commit_estimate = 0; lfs_size_t commit_estimate = 0;
for (lfs_size_t i = 0; i < attr_count; i++) { for (lfs_size_t i = 0; i < attr_count; i++) {
// only include tag overhead if tag is not a grow tag // only include tag overhead if tag is not a grow/rm tag
if (!lfsr_tag_isgrow(attrs[i].tag)) { if (!lfsr_tag_isgrow(attrs[i].tag)
&& !lfsr_tag_isrm(attrs[i].tag)) {
commit_estimate += LFSR_ATTR_ESTIMATE; commit_estimate += LFSR_ATTR_ESTIMATE;
} }
commit_estimate += lfsr_data_size(&attrs[i].data); commit_estimate += lfsr_data_size(&attrs[i].data);
} }
// avoid some overflow issues here // does our estimate exceed our shrub_size? need to recalculate an
// accurate estimate
lfs_ssize_t estimate = (alloc) lfs_ssize_t estimate = (alloc)
? (lfs_size_t)-1 ? (lfs_size_t)-1
: file->ftree.u.bshrub.estimate; : file->ftree.u.bshrub.estimate;
if ((lfs_size_t)estimate <= lfs->cfg->shrub_size) { // this double condition avoids overflow issues
estimate += commit_estimate; if ((lfs_size_t)estimate > lfs->cfg->shrub_size
} || estimate + commit_estimate > lfs->cfg->shrub_size) {
// does our estimate exceed our shrub_size? need to recalculate an
// accurate our estimate
if ((lfs_size_t)estimate > lfs->cfg->shrub_size) {
estimate = lfsr_file_estimate(lfs, file); estimate = lfsr_file_estimate(lfs, file);
if (estimate < 0) { if (estimate < 0) {
return estimate; return estimate;
} }
// TODO defer this? // two cases where we evict:
// don't forget to include our pending commit // - overlow shrub_size/2 - don't penalize for commits here
estimate += commit_estimate; // - overlow shrub_size - must include commits or we risk overflow
//
// do we overflow shrub_size/2? the 1/2 here prevents runaway // the 1/2 here prevents runaway performance with the shrub is
// performance when the shrub is near full // near full, but it's a heuristic, so including the commit would
if ((lfs_size_t)estimate > lfs->cfg->shrub_size/2) { // just be mean
//
if ((lfs_size_t)estimate > lfs->cfg->shrub_size/2
|| estimate + commit_estimate > lfs->cfg->shrub_size) {
goto evict; goto evict;
} }
} }
// include our pending commit in the new estimate
estimate += commit_estimate;
// commit to shrub // commit to shrub
int err = lfsr_mdir_commit(lfs, &file->mdir, LFSR_ATTRS( int err = lfsr_mdir_commit(lfs, &file->mdir, LFSR_ATTRS(
LFSR_ATTR(file->mdir.mid, LFSR_ATTR(file->mdir.mid,