Skip to content

Commit

Permalink
Improve performance of flushing single objects (HDFGroup#4017)
Browse files Browse the repository at this point in the history 
Improve performance of flushing a single object, and remove metadata
cache flush markers
  • Loading branch information
@fortnern
fortnern authored Feb 23, 2024
1 parent 3fd1e90 commit 560e80c
Show file tree
Hide file tree
Showing 11 changed files with 2,627 additions and 4,257 deletions.
4 changes: 2 additions & 2 deletions src/H5AC.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ H5AC_dest(H5F_t *f)
*/
if (H5F_ACC_RDWR & H5F_INTENT(f)) {
/* enable and load the skip list */
if (H5C_set_slist_enabled(f->shared->cache, true, false) < 0)
if (H5C_set_slist_enabled(f->shared->cache, true, true) < 0)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "can't enable skip list");

if (H5AC__flush_entries(f) < 0)
Expand Down Expand Up @@ -1127,7 +1127,7 @@ H5AC_prep_for_file_flush(H5F_t *f)
assert(f->shared);
assert(f->shared->cache);

if (H5C_set_slist_enabled(f->shared->cache, true, false) < 0)
if (H5C_set_slist_enabled(f->shared->cache, true, true) < 0)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "can't enable skip list");

done:
Expand Down
2 changes: 0 additions & 2 deletions src/H5ACprivate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,14 +312,12 @@ typedef struct H5AC_proxy_entry_t {
*/

#define H5AC__NO_FLAGS_SET H5C__NO_FLAGS_SET
#define H5AC__SET_FLUSH_MARKER_FLAG H5C__SET_FLUSH_MARKER_FLAG
#define H5AC__DELETED_FLAG H5C__DELETED_FLAG
#define H5AC__DIRTIED_FLAG H5C__DIRTIED_FLAG
#define H5AC__PIN_ENTRY_FLAG H5C__PIN_ENTRY_FLAG
#define H5AC__UNPIN_ENTRY_FLAG H5C__UNPIN_ENTRY_FLAG
#define H5AC__FLUSH_INVALIDATE_FLAG H5C__FLUSH_INVALIDATE_FLAG
#define H5AC__FLUSH_CLEAR_ONLY_FLAG H5C__FLUSH_CLEAR_ONLY_FLAG
#define H5AC__FLUSH_MARKED_ENTRIES_FLAG H5C__FLUSH_MARKED_ENTRIES_FLAG
#define H5AC__FLUSH_IGNORE_PROTECTED_FLAG H5C__FLUSH_IGNORE_PROTECTED_FLAG
#define H5AC__READ_ONLY_FLAG H5C__READ_ONLY_FLAG
#define H5AC__FREE_FILE_SPACE_FLAG H5C__FREE_FILE_SPACE_FLAG
Expand Down
79 changes: 30 additions & 49 deletions src/H5C.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ H5C_dest(H5F_t *f)
#endif /* H5AC_DUMP_IMAGE_STATS_ON_CLOSE */

/* Enable the slist, as it is needed in the flush */
if (H5C_set_slist_enabled(f->shared->cache, true, false) < 0)
if (H5C_set_slist_enabled(f->shared->cache, true, true) < 0)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "set slist enabled failed");

/* Flush and invalidate all cache entries */
Expand Down Expand Up @@ -567,15 +567,15 @@ H5C_evict(H5F_t *f)
assert(f);

/* Enable the slist, as it is needed in the flush */
if (H5C_set_slist_enabled(f->shared->cache, true, false) < 0)
if (H5C_set_slist_enabled(f->shared->cache, true, true) < 0)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "set slist enabled failed");

/* Flush and invalidate all cache entries except the pinned entries */
if (H5C__flush_invalidate_cache(f, H5C__EVICT_ALLOW_LAST_PINS_FLAG) < 0)
HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to evict entries in the cache");

/* Disable the slist */
if (H5C_set_slist_enabled(f->shared->cache, false, true) < 0)
if (H5C_set_slist_enabled(f->shared->cache, false, false) < 0)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "set slist disabled failed");

done:
Expand Down Expand Up @@ -1042,41 +1042,32 @@ H5C_set_evictions_enabled(H5C_t *cache_ptr, bool evictions_enabled)
*
* 1) Verifies that the slist is empty.
*
* 2) Scans the index list, and inserts all dirty entries
* into the slist.
* 2) If the populate_slist parameter is true, scans the
* index list, and inserts all dirty entries into the
* slist.
*
* 3) Sets cache_ptr->slist_enabled = true.
*
* Note that the clear_slist parameter is ignored if
* the slist_enabed parameter is true.
*
*
* If the slist_enabled_parameter is false, the function
* shuts down the slist.
*
* Normally the slist will be empty at this point, however
* that need not be the case if H5C_flush_cache() has been
* called with the H5C__FLUSH_MARKED_ENTRIES_FLAG.
*
* Thus shutdown proceeds as follows:
* shuts down the slist:
*
* 1) Test to see if the slist is empty. If it is, proceed
* to step 3.
*
* 2) Test to see if the clear_slist parameter is true.
*
* If it is, remove all entries from the slist.
*
* If it isn't, throw an error.
* 2) Remove all entries from the slist.
*
* 3) set cache_ptr->slist_enabled = false.
*
* Note that the populate_slist parameter is ignored if
* the slist_enabed parameter is false.
*
* Return: SUCCEED on success, and FAIL on failure.
*
*-------------------------------------------------------------------------
*/
herr_t
H5C_set_slist_enabled(H5C_t *cache_ptr, bool slist_enabled, bool clear_slist)
H5C_set_slist_enabled(H5C_t *cache_ptr, bool slist_enabled, bool populate_slist)
{
H5C_cache_entry_t *entry_ptr;
herr_t ret_value = SUCCEED; /* Return value */
Expand All @@ -1097,40 +1088,30 @@ H5C_set_slist_enabled(H5C_t *cache_ptr, bool slist_enabled, bool clear_slist)
*/
cache_ptr->slist_enabled = true;

/* scan the index list and insert all dirty entries in the slist */
entry_ptr = cache_ptr->il_head;
while (entry_ptr != NULL) {
if (entry_ptr->is_dirty)
H5C__INSERT_ENTRY_IN_SLIST(cache_ptr, entry_ptr, FAIL);
entry_ptr = entry_ptr->il_next;
}
if (populate_slist) {
/* scan the index list and insert all dirty entries in the slist */
entry_ptr = cache_ptr->il_head;
while (entry_ptr != NULL) {
if (entry_ptr->is_dirty)
H5C__INSERT_ENTRY_IN_SLIST(cache_ptr, entry_ptr, FAIL);
entry_ptr = entry_ptr->il_next;
}

/* we don't maintain a dirty index len, so we can't do a cross
* check against it. Note that there is no point in cross checking
* against the dirty LRU size, as the dirty LRU may not be maintained,
* and in any case, there is no requirement that all dirty entries
* will reside on the dirty LRU.
*/
assert(cache_ptr->dirty_index_size == cache_ptr->slist_size);
/* we don't maintain a dirty index len, so we can't do a cross
* check against it. Note that there is no point in cross checking
* against the dirty LRU size, as the dirty LRU may not be maintained,
* and in any case, there is no requirement that all dirty entries
* will reside on the dirty LRU.
*/
assert(cache_ptr->dirty_index_size == cache_ptr->slist_size);
}
}
else { /* take down the skip list */
if (!cache_ptr->slist_enabled)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "slist already disabled?");

if ((cache_ptr->slist_len != 0) || (cache_ptr->slist_size != 0)) {
if (clear_slist) {
H5SL_node_t *node_ptr;

node_ptr = H5SL_first(cache_ptr->slist_ptr);
while (node_ptr != NULL) {
entry_ptr = (H5C_cache_entry_t *)H5SL_item(node_ptr);
H5C__REMOVE_ENTRY_FROM_SLIST(cache_ptr, entry_ptr, false, FAIL);
node_ptr = H5SL_first(cache_ptr->slist_ptr);
}
}
else
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "slist not empty?");
}
if ((cache_ptr->slist_len != 0) || (cache_ptr->slist_size != 0))
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "slist not empty?");

cache_ptr->slist_enabled = false;

Expand Down
58 changes: 15 additions & 43 deletions src/H5Centry.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,36 +503,24 @@ H5C__flush_single_entry(H5F_t *f, H5C_cache_entry_t *entry_ptr, unsigned flags)
if (cache_ptr->slist_enabled) {
if (entry_ptr->in_slist) {
assert(entry_ptr->is_dirty);
if (entry_ptr->flush_marker && !entry_ptr->is_dirty)
if (!entry_ptr->is_dirty)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "entry in slist failed sanity checks");
} /* end if */
else {
assert(!entry_ptr->is_dirty);
assert(!entry_ptr->flush_marker);
if (entry_ptr->is_dirty || entry_ptr->flush_marker)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "entry failed sanity checks");
} /* end else */
}
else { /* slist is disabled */
else /* slist is disabled */
assert(!entry_ptr->in_slist);
if (!entry_ptr->is_dirty)
if (entry_ptr->flush_marker)
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "flush marked clean entry?");
}
#endif /* H5C_DO_SANITY_CHECKS */

if (entry_ptr->is_protected)
/* Attempt to flush a protected entry -- scream and die. */
HGOTO_ERROR(H5E_CACHE, H5E_PROTECT, FAIL, "Attempt to flush a protected entry");

/* Set entry_ptr->flush_in_progress = true and set
* entry_ptr->flush_marker = false
/* Set entry_ptr->flush_in_progress = true
*
* We will set flush_in_progress back to false at the end if the
* entry still exists at that point.
*/
entry_ptr->flush_in_progress = true;
entry_ptr->flush_marker = false;

/* Preserve current dirty state for later */
was_dirty = entry_ptr->is_dirty;
Expand Down Expand Up @@ -1240,7 +1228,6 @@ H5C__load_entry(H5F_t *f,
entry->ro_ref_count = 0;
entry->is_pinned = false;
entry->in_slist = false;
entry->flush_marker = false;
#ifdef H5_HAVE_PARALLEL
entry->clear_on_unprotect = false;
entry->flush_immediately = false;
Expand Down Expand Up @@ -1897,7 +1884,6 @@ H5C__deserialize_prefetched_entry(H5F_t *f, H5C_t *cache_ptr, H5C_cache_entry_t
ds_entry_ptr->ro_ref_count = 0;
ds_entry_ptr->is_pinned = false;
ds_entry_ptr->in_slist = false;
ds_entry_ptr->flush_marker = false;
#ifdef H5_HAVE_PARALLEL
ds_entry_ptr->clear_on_unprotect = false;
ds_entry_ptr->flush_immediately = false;
Expand Down Expand Up @@ -2095,7 +2081,6 @@ H5C_insert_entry(H5F_t *f, const H5C_class_t *type, haddr_t addr, void *thing, u
#ifdef H5_HAVE_PARALLEL
bool coll_access = false; /* whether access to the cache entry is done collectively */
#endif /* H5_HAVE_PARALLEL */
bool set_flush_marker;
bool write_permitted = true;
size_t empty_space;
H5C_cache_entry_t *entry_ptr = NULL;
Expand Down Expand Up @@ -2125,9 +2110,8 @@ H5C_insert_entry(H5F_t *f, const H5C_class_t *type, haddr_t addr, void *thing, u
HGOTO_ERROR(H5E_CACHE, H5E_SYSTEM, FAIL, "an extreme sanity check failed on entry");
#endif /* H5C_DO_EXTREME_SANITY_CHECKS */

set_flush_marker = ((flags & H5C__SET_FLUSH_MARKER_FLAG) != 0);
insert_pinned = ((flags & H5C__PIN_ENTRY_FLAG) != 0);
flush_last = ((flags & H5C__FLUSH_LAST_FLAG) != 0);
insert_pinned = ((flags & H5C__PIN_ENTRY_FLAG) != 0);
flush_last = ((flags & H5C__FLUSH_LAST_FLAG) != 0);

/* Get the ring type from the API context */
ring = H5CX_get_ring();
Expand Down Expand Up @@ -2301,7 +2285,6 @@ H5C_insert_entry(H5F_t *f, const H5C_class_t *type, haddr_t addr, void *thing, u

/* New entries are presumed to be dirty */
assert(entry_ptr->is_dirty);
entry_ptr->flush_marker = set_flush_marker;
H5C__INSERT_ENTRY_IN_SLIST(cache_ptr, entry_ptr, FAIL);
H5C__UPDATE_RP_FOR_INSERTION(cache_ptr, entry_ptr, FAIL);

Expand Down Expand Up @@ -2497,9 +2480,6 @@ H5C_mark_entry_clean(void *_thing)
/* Mark the entry as clean if it isn't already */
entry_ptr->is_dirty = false;

/* Also reset the 'flush_marker' flag, since the entry shouldn't be flushed now */
entry_ptr->flush_marker = false;

/* Modify cache data structures */
if (was_dirty)
H5C__UPDATE_INDEX_FOR_ENTRY_CLEAN(cache_ptr, entry_ptr, FAIL);
Expand Down Expand Up @@ -3426,7 +3406,6 @@ H5C_unprotect(H5F_t *f, haddr_t addr, void *thing, unsigned flags)
H5C_t *cache_ptr;
bool deleted;
bool dirtied;
bool set_flush_marker;
bool pin_entry;
bool unpin_entry;
bool free_file_space;
Expand All @@ -3441,13 +3420,12 @@ H5C_unprotect(H5F_t *f, haddr_t addr, void *thing, unsigned flags)

FUNC_ENTER_NOAPI(FAIL)

deleted = ((flags & H5C__DELETED_FLAG) != 0);
dirtied = ((flags & H5C__DIRTIED_FLAG) != 0);
set_flush_marker = ((flags & H5C__SET_FLUSH_MARKER_FLAG) != 0);
pin_entry = ((flags & H5C__PIN_ENTRY_FLAG) != 0);
unpin_entry = ((flags & H5C__UNPIN_ENTRY_FLAG) != 0);
free_file_space = ((flags & H5C__FREE_FILE_SPACE_FLAG) != 0);
take_ownership = ((flags & H5C__TAKE_OWNERSHIP_FLAG) != 0);
deleted = ((flags & H5C__DELETED_FLAG) != 0);
dirtied = ((flags & H5C__DIRTIED_FLAG) != 0);
pin_entry = ((flags & H5C__PIN_ENTRY_FLAG) != 0);
unpin_entry = ((flags & H5C__UNPIN_ENTRY_FLAG) != 0);
free_file_space = ((flags & H5C__FREE_FILE_SPACE_FLAG) != 0);
take_ownership = ((flags & H5C__TAKE_OWNERSHIP_FLAG) != 0);

assert(f);
assert(f->shared);
Expand Down Expand Up @@ -3621,15 +3599,10 @@ H5C_unprotect(H5F_t *f, haddr_t addr, void *thing, unsigned flags)

entry_ptr->is_protected = false;

/* if the entry is dirty, 'or' its flush_marker with the set flush flag,
* and then add it to the skip list if it isn't there already.
*/
if (entry_ptr->is_dirty) {
entry_ptr->flush_marker |= set_flush_marker;
if (!entry_ptr->in_slist)
/* this is a no-op if cache_ptr->slist_enabled is false */
H5C__INSERT_ENTRY_IN_SLIST(cache_ptr, entry_ptr, FAIL);
} /* end if */
/* if the entry is dirty, add it to the skip list if it isn't there already. */
if (entry_ptr->is_dirty && !entry_ptr->in_slist)
/* this is a no-op if cache_ptr->slist_enabled is false */
H5C__INSERT_ENTRY_IN_SLIST(cache_ptr, entry_ptr, FAIL);

/* This implementation of the "deleted" option is a bit inefficient, as
* we re-insert the entry to be deleted into the replacement policy
Expand Down Expand Up @@ -4141,7 +4114,6 @@ H5C_remove_entry(void *_entry)

/* Additional internal cache consistency checks */
assert(!entry->in_slist);
assert(!entry->flush_marker);
assert(!entry->flush_in_progress);

/* Note that the algorithm below is (very) similar to the set of operations
Expand Down
Loading

0 comments on commit 560e80c

Please sign in to comment.