Skip to content

Fix C-style cast in Cache subsystem - #11733

Merged
masaori335 merged 4 commits into
apache:masterfrom
masaori335:asf-master-stripe-1
Aug 28, 2024
Merged

Fix C-style cast in Cache subsystem#11733
masaori335 merged 4 commits into
apache:masterfrom
masaori335:asf-master-stripe-1

Conversation

@masaori335

Copy link
Copy Markdown
Contributor

Will be rebased when other cleanup PRs are merged.

@masaori335 masaori335 added this to the 10.1.0 milestone Aug 22, 2024
@masaori335 masaori335 self-assigned this Aug 22, 2024
Comment thread src/iocore/cache/CacheDir.cc
@masaori335
masaori335 force-pushed the asf-master-stripe-1 branch from 10901ff to 099f599 Compare August 23, 2024 02:17
Comment thread src/iocore/cache/CacheVC.cc Outdated
@masaori335
masaori335 marked this pull request as ready for review August 23, 2024 02:22

@JosiahWI JosiahWI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for updating the casts. Could we update all the casts in the whole subsystem together? I found some more casts with grep -r '([a-z]\+)(' src/iocore/cache.

(long)(cachep->size * STORE_BLOCK_SIZE));

#define CACHE_VALUE_HITS_SIZE(_h, _s) ((float)((_h) + 1) / ((_s) + ENTRY_OVERHEAD))

And also:

jvanderzee@V22MYH9793 trafficserver % grep -r '([a-z]\+)[a-z]' src/iocore/cache
src/iocore/cache/CacheProcessor.cc:  ink_assert(rindex == (int)rtable_size);
src/iocore/cache/CacheProcessor.cc:          ink_assert(p->scheme == (int)dp[j]->dpb_queue.head->b->type);
src/iocore/cache/unit_tests/test_CacheVol.cc:      ink_assert(blocks <= (int)total_space);
src/iocore/cache/CacheDir.cc:  DDbg(dbg_ctl_dir_lookaside, "insert %X %X, offset %d phase %d", key->slice32(0), key->slice32(1), (int)dir_offset(to),
src/iocore/cache/CacheDir.cc:       (int)dir_phase(to));
src/iocore/cache/P_CacheInternal.h:  (void)event;
src/iocore/cache/P_CacheInternal.h:  (void)data;
src/iocore/cache/StripeSM.cc:      (long long)this->len, (double)dirlen() / (double)this->len * 100.0);
src/iocore/cache/StripeSM.cc:      DDbg(dbg_ctl_cache_evac, "evac_range evacuating %X %d", (int)dir_tag(&first->dir), (int)dir_offset(&first->dir));
src/iocore/cache/StripeSM.cc:    Dbg(dbg_ctl_cache_evac, "DOC magic: %X %d", (int)dir_tag(&doc_evacuator->overwrite_dir),
src/iocore/cache/StripeSM.cc:        (int)dir_offset(&doc_evacuator->overwrite_dir));
src/iocore/cache/StripeSM.cc:  DDbg(dbg_ctl_cache_evac, "evacuateDocReadDone %X offset %d", (int)doc->key.slice32(0),
src/iocore/cache/StripeSM.cc:       (int)dir_offset(&doc_evacuator->overwrite_dir));
src/iocore/cache/StripeSM.cc:      DDbg(dbg_ctl_cache_evac, "evacuating vector %X offset %d", (int)doc->first_key.slice32(0),
src/iocore/cache/StripeSM.cc:           (int)dir_offset(&doc_evacuator->overwrite_dir));
src/iocore/cache/StripeSM.cc:      DDbg(dbg_ctl_cache_evac, "evacuating earliest %X %X evac: %p offset: %d", (int)b->evac_frags.key.slice32(0),
src/iocore/cache/StripeSM.cc:           (int)doc->key.slice32(0), doc_evacuator, (int)dir_offset(&doc_evacuator->overwrite_dir));
src/iocore/cache/StripeSM.cc:    DDbg(dbg_ctl_cache_evac, "evacuateDocReadDone key: %X earliest: %X", (int)ek->key.slice32(0), (int)ek->earliest_key.slice32(0));
src/iocore/cache/StripeSM.cc:    DDbg(dbg_ctl_cache_evac, "next fragment %X Earliest: %X offset %d phase %d force %d", (int)key->slice32(0),
src/iocore/cache/StripeSM.cc:         (int)earliest_key->slice32(0), (int)dir_offset(&dir), (int)dir_phase(&dir), force);
src/iocore/cache/CacheEvacuateDocVC.cc:  DDbg(dbg_ctl_cache_evac, "evacuateDocDone %X o %d p %d new_o %d new_p %d", (int)key.slice32(0),
src/iocore/cache/CacheEvacuateDocVC.cc:       (int)dir_offset(&this->overwrite_dir), (int)dir_phase(&this->overwrite_dir), (int)dir_offset(&this->dir),
src/iocore/cache/CacheEvacuateDocVC.cc:       (int)dir_phase(&this->dir));
src/iocore/cache/CacheEvacuateDocVC.cc:      DDbg(dbg_ctl_cache_evac, "evacuateDocDone evacuate_head %X %X hlen %d offset %d", (int)key.slice32(0),
src/iocore/cache/CacheEvacuateDocVC.cc:           (int)doc->key.slice32(0), doc->hlen, (int)dir_offset(&this->overwrite_dir));
src/iocore/cache/CacheEvacuateDocVC.cc:        DDbg(dbg_ctl_cache_evac, "evacuating vector: %X %d", (int)doc->first_key.slice32(0), (int)dir_offset(&this->overwrite_dir));
src/iocore/cache/CacheEvacuateDocVC.cc:          DDbg(dbg_ctl_cache_evac, "overwriting the open directory %X %d %d", (int)doc->first_key.slice32(0),
src/iocore/cache/CacheEvacuateDocVC.cc:               (int)dir_offset(&cod->first_dir), (int)dir_offset(&this->dir));
src/iocore/cache/CacheEvacuateDocVC.cc:        DDbg(dbg_ctl_cache_evac, "evacuating earliest: %X %d", (int)doc->key.slice32(0), (int)dir_offset(&this->overwrite_dir));
src/iocore/cache/PreservationTable.cc:         (int)dir_offset(evac_dir), (int)dir_phase(evac_dir));
src/iocore/cache/PreservationTable.cc:    DDbg(dbg_ctl_cache_evac, "force: %d, %d", (int)dir_offset(evac_dir), (int)dir_phase(evac_dir));
src/iocore/cache/PreservationTable.cc:      DDbg(dbg_ctl_cache_evac, "evacuate cleanup free %X offset %d", (int)b->evac_frags.key.slice32(0), (int)dir_offset(&b->dir));
src/iocore/cache/CacheRead.cc:         first_key.slice32(1), write_vc->closed, write_vc->fragment, (int)doc_len);
src/iocore/cache/CacheRead.cc:    Warning("Document %X truncated at %d of %d, reading from writer", first_key.slice32(1), (int)vio.ndone, (int)doc_len);
src/iocore/cache/CacheRead.cc:            DDbg(dbg_ctl_cache_read_agg, "%p: key: %X ReadRead complete: %d", this, first_key.slice32(1), (int)vio.ndone);
src/iocore/cache/CacheRead.cc:        DDbg(dbg_ctl_cache_read_agg, "%p: key: %X ReadRead writer aborted: %d", this, first_key.slice32(1), (int)vio.ndone);
src/iocore/cache/CacheRead.cc:        DDbg(dbg_ctl_cache_read_agg, "%p: key: %X ReadRead retrying: %d", this, first_key.slice32(1), (int)vio.ndone);
src/iocore/cache/CacheRead.cc:             (int)vio.ndone);
src/iocore/cache/CacheRead.cc:          DDbg(dbg_ctl_cache_read_agg, "%p: key: %X ReadMain complete: %d", this, first_key.slice32(1), (int)vio.ndone);
src/iocore/cache/CacheRead.cc:      DDbg(dbg_ctl_cache_read_agg, "%p: key: %X ReadMain writer aborted: %d", this, first_key.slice32(1), (int)vio.ndone);
src/iocore/cache/CacheRead.cc:    DDbg(dbg_ctl_cache_read_agg, "%p: key: %X ReadMain retrying: %d", this, first_key.slice32(1), (int)vio.ndone);
src/iocore/cache/CacheRead.cc:  Warning("Document %X truncated at %d of %d, missing fragment %X", first_key.slice32(1), (int)vio.ndone, (int)doc_len,
src/iocore/cache/CacheVC.cc:  (void)avio;
src/iocore/cache/CacheVC.cc:  (void)avio;
src/iocore/cache/CacheVC.cc:      (void)e; // Avoid compiler warnings
src/iocore/cache/Cache.cc:      (int)cache_config_min_average_object_size);
src/iocore/cache/AggregateWriteBuffer.cc:  ink_assert((offset + nbytes) <= (unsigned)this->_buffer_pos);
src/iocore/cache/P_CacheHosting.h:    (void)e;
src/iocore/cache/P_CacheHosting.h:    (void)event;

Comment thread src/iocore/cache/CacheProcessor.cc Outdated
Comment thread src/iocore/cache/Stripe.h Outdated
@masaori335

Copy link
Copy Markdown
Contributor Author

Nice catch! My clang-tidy and clangd missed them. I'll fix them.

@JosiahWI
JosiahWI self-requested a review August 26, 2024 22:04
Comment thread src/iocore/cache/CacheProcessor.cc Outdated
Comment thread src/iocore/cache/CacheVC.cc Outdated
@masaori335
masaori335 merged commit 6f06307 into apache:master Aug 28, 2024
bryancall pushed a commit that referenced this pull request Jul 13, 2026
PR #11733 rewrote the CACHE_VALUE_HITS_SIZE cast so static_cast<float>
wraps the whole quotient, making (hits + 1) / (size + overhead) integer
division. It truncates to 0 for normal object sizes, zeroing the value
metric and collapsing CLFUS to FIFO: no promote-on-hit, no clock second
chance, and no value-based ghost re-admission.

Bind the cast to the numerator to restore floating-point division, and
add the ram_cache_clfus_value regression test as a guard (it fails on
the pre-fix macro and passes after).
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jul 29, 2026
…3233)

PR apache#11733 rewrote the CACHE_VALUE_HITS_SIZE cast so static_cast<float>
wraps the whole quotient, making (hits + 1) / (size + overhead) integer
division. It truncates to 0 for normal object sizes, zeroing the value
metric and collapsing CLFUS to FIFO: no promote-on-hit, no clock second
chance, and no value-based ghost re-admission.

Bind the cast to the numerator to restore floating-point division, and
add the ram_cache_clfus_value regression test as a guard (it fails on
the pre-fix macro and passes after).
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jul 29, 2026
…3233)

PR apache#11733 rewrote the CACHE_VALUE_HITS_SIZE cast so static_cast<float>
wraps the whole quotient, making (hits + 1) / (size + overhead) integer
division. It truncates to 0 for normal object sizes, zeroing the value
metric and collapsing CLFUS to FIFO: no promote-on-hit, no clock second
chance, and no value-based ghost re-admission.

Bind the cast to the numerator to restore floating-point division, and
add the ram_cache_clfus_value regression test as a guard (it fails on
the pre-fix macro and passes after).

(cherry picked from commit d1d02c3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants