[upstream] use getCacheSize() in shm construction #48
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vinser52
requested changes
Jan 4, 2023
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @byrnedj and @igchor)
cachelib/allocator/CacheAllocator-inl.h line 138 at r1 (raw file):
shmManager_ ->attachShm(detail::kShmCacheName, config_.slabMemoryBaseAddr, createShmCacheOpts())
I think you should keep initial formating and just replace config_.size -> config_.getCacheSize()
igchor
approved these changes
Jan 4, 2023
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @byrnedj)
byrnedj
force-pushed
the
calculate_size
branch
from
496959e to
4c408b4
Compare
byrnedj
commented
Jan 4, 2023
There was a problem hiding this comment.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @vinser52)
cachelib/allocator/CacheAllocator-inl.h line 138 at r1 (raw file):
Previously, vinser52 (Sergei Vinogradov) wrote…
I think you should keep initial formating and just replace
config_.size->config_.getCacheSize()
Done.
vinser52
approved these changes
Jan 4, 2023
There was a problem hiding this comment.
Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @byrnedj)
vinser52
reviewed
Jan 4, 2023
There was a problem hiding this comment.
Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @byrnedj)
igchor
reviewed
Jan 4, 2023
There was a problem hiding this comment.
clang-format should be fixed
Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @vinser52)
byrnedj
force-pushed
the
calculate_size
branch
from
4c408b4 to
9b3b19e
Compare
Summary: GitHub commits: facebook/wangle@43e5620 facebookexperimental/edencommon@3934bec facebookincubator/katran@a4c94aa Reviewed By: jurajh-fb fbshipit-source-id: 720310662068e4aa245198d817666700c9dde1d5
Summary: GitHub commits: facebook/fb303@c51a290 facebook/fbthrift@5e97023 facebook/proxygen@3710a32 facebook/wangle@fd29800 facebook/watchman@fca53d2 facebookexperimental/edencommon@3d8a6d9 facebookexperimental/rust-shed@75c5d75 facebookincubator/fizz@bae2d7e facebookincubator/katran@139c75d facebook/mvfst@001332d facebookincubator/velox@ec0ea1f Reviewed By: jurajh-fb fbshipit-source-id: a8a8ff6a7fddeaa7bbccb3faa6a737c50d22cfe2
Summary: As mentioned in the tasks, these stats make more sense in rates. Reviewed By: jiayuebao Differential Revision: D43243498 fbshipit-source-id: 556b0b4a062235bb3b34dd076e5f7a5a88316f8c
Summary: GitHub commits: facebook@ba9dd68 facebook/fbthrift@23208ff facebookincubator/velox@68b8a45 Reviewed By: jurajh-fb fbshipit-source-id: f8581e13cd1c0a2510d87f42c813ca2c65eaef1d
Summary: GitHub commits: facebook/fbthrift@3b00d58 Reviewed By: jurajh-fb fbshipit-source-id: 23ecb07e7e9ebd4dfbf3980de141ea1ebe36bb12
Summary: GitHub commits: facebook/fbthrift@df0eb61 facebook/proxygen@7fc3fea Reviewed By: jurajh-fb fbshipit-source-id: fa982baf35fd98edc52be391f03d15a9c2c333bf
Summary: GitHub commits: facebook/fbthrift@1d01d76 Reviewed By: jurajh-fb fbshipit-source-id: bdefba88e8d002c65d8b7c11e86c8aa772996d9b
Summary: GitHub commits: facebook/fbthrift@1b848d8 facebook/rocksdb@cfe50f7 pytorch/FBGEMM@c55d800 Reviewed By: bigfootjon fbshipit-source-id: e840d1116311ac52336e598442cf66cdc50d7725
Summary: GitHub commits: facebook/fbthrift@90fddab Reviewed By: bigfootjon fbshipit-source-id: cf7ac48c00c548805fb74663fe3e9bd9f5906cc2
Summary: Currently `replacedPtr` is passed as an argument and such usage isn't easy to understand (we kept receiving user feedback on that). Now we want to return it. Reviewed By: therealgymmy, antonf, jaesoo-fb Differential Revision: D43143989 fbshipit-source-id: eaae055107050cd67a7ca8b26b716fffc0c002f2
Summary: GitHub commits: facebook/folly@fdb333b facebook/watchman@0b238b0 pytorch/FBGEMM@d142cdf Reviewed By: bigfootjon fbshipit-source-id: ac185dc9dc24ba7d2765dca010958ce6b38bd029
Summary: GitHub commits: facebook/fbthrift@cbc3de5 facebook/rocksdb@229297d facebook/watchman@d873e11 facebookexperimental/rust-shed@2f45b88 Reviewed By: bigfootjon fbshipit-source-id: a65fa2098f6b6f1704154e0509e2f5423f9679eb
Summary: Change build script to pin fmt version to same as folly's to minimize future breaks. Pull Request resolved: facebook#196 Test Plan: Built successfully on a fresh clone of CacheLib. (Also had to change `external_git_branch=dev` for zstd to deal with the cmake/zstd issue in facebook#194, but that should resolve when gets merged into release) **Context:** OSS build broke between 3-5 Jan 2023, likely due to changes in folly. While switching to v9.1.0 or 9.0.0 fixes the issue at hand, it seems sensible to match folly, which specifies fmt v8.0.1: https://github.com/facebook/folly/blob/main/build/fbcode_builder/manifests/fmt > facebook#62 agordon: For the other packages, you'll notice we do use a specific git tag or branch… I notice `fmt` is an exception - not pinned to a specific git tag or revision - likely an omission that can be fixed. Related CacheLib issues: facebook#186, facebook#189, facebook#107, facebook#97, facebook#62 Possibly related CacheLib commit: 67cc11a Last working (Jan 3): https://github.com/facebook/CacheLib/actions/runs/3826992478 First failed (Jan 5): https://github.com/facebook/CacheLib/actions/runs/3844002307/jobs/6546742348 Error: `error: static assertion failed: Cannot format an argument. To make type T formattable provide a formatter<T> specialization: https://fmt.dev/latest/api.html#udt` Reviewed By: therealgymmy Differential Revision: D43517927 Pulled By: jiayuebao fbshipit-source-id: 2d28791f7804d862b646263b96b10b835f843d8c
Summary: Memcached's WSA logger will now emits GET_LEASE and SET_LEASE operations as well. This changes makes the cachebench treats those as GET and SET, respectively, for compatibility. Reviewed By: therealgymmy Differential Revision: D43336316 fbshipit-source-id: c9b842d567b9fb2128b822bd429f5dce30b378da
Summary: update to >6.0.1 version Reviewed By: antonk52 Differential Revision: D43575577 fbshipit-source-id: 7143da212ab6f124bffdfdaf3be29ff3ab986ffb
Summary: Fix OSS builds by adding numa deps to build files. Currently some fail on missing `numa.h`. Context: facebook#161 added the dependencies to the centOS, debian, and ubuntu18 build files. The PR was opened in Sep 2022 but only landed in Dec 2022, and so probably missed out on the fedora, rocky and arch build files which were added in-between those dates. Having had those build actions run on PRs would have caught this (currently, they are only scheduled.) Pull Request resolved: facebook#197 Test Plan: Github Actions builds (ideally, facebook#198 would be landed first.) I've checked that those packages exist for the respective repositories but didn't run them myself. Reviewed By: jaesoo-fb Differential Revision: D43587970 Pulled By: jiayuebao fbshipit-source-id: 8c59e48528042350e576a45ffc3bf2520699f5a9
Reviewed By: avalonalex Differential Revision: D43616310 fbshipit-source-id: 3367ad01ba31e5dc561d63a4f3b9746170e64912
Summary: Run GitHub action builds on every pull request, in addition to currently daily scheduled runs. Benefit: avoid accidentally breaking other OS builds. This would have caught facebook#197. This triggers whenever PRs are opened or when commits are added to a PR. Frequency of PRs (total of 76 PRs over last 18 months since CacheLib was open-sourced) is much less than frequency of daily scheduled builds, so this shouldn't add too many builds overall. Pull Request resolved: facebook#198 Reviewed By: therealgymmy Differential Revision: D43625609 Pulled By: jiayuebao fbshipit-source-id: 1572f6da32584ce6a1983d5e64afedf17ff17457
Summary: Add a custom deleter class that stores a `handle`. This allows object-cache to access the Item Handle via a shared_ptr. Both size-awareness feature and getting/updating object's TTL require that. Deleter class is marked as private because we don't want to expose `Handle` to object-cache users. Reviewed By: therealgymmy, jaesoo-fb Differential Revision: D42503594 fbshipit-source-id: 16ac14e6a84a1cfa80a3c145d440790002734a34
Summary: GitHub commits: facebook/fb303@5dada62 facebook/fbthrift@5782f94 facebook/litho@ef3b93e facebook/proxygen@9a63096 facebook/wangle@8e88e60 facebook/watchman@d2fb403 facebookexperimental/edencommon@e04a87f facebookexperimental/rust-shed@e638a99 facebookincubator/fizz@8d6e1c7 facebookincubator/katran@14c83d3 facebook/mvfst@4f24529 facebookincubator/velox@404237b Reviewed By: bigfootjon fbshipit-source-id: 25c61fba0e567a9dc87f6cc7dc5ba7bb0e0ccdaa
Summary: Previously, in order to use a file as the backing store of nvm cache, user needs to create the file with the given size a priori, hurting the usability of the feature. This change fixes it so that the new file with the given size is automatically created if not exist. Also, if the file exists with different size, this change will allow the cachebench to ftruncate or fallocate to the given size automatically. Reviewed By: therealgymmy Differential Revision: D44154281 fbshipit-source-id: c622102d4d67b64da83dcad4fdade66473d70a1f
Summary: GitHub commits: facebook@02ef4a1 facebook/fbthrift@721caa7 facebook/folly@60ffa0f facebook/litho@47e41c0 facebookincubator/Eigen-FBPlugins@742056b facebookincubator/velox@8b883b0 Reviewed By: bigfootjon fbshipit-source-id: 299df33a51a79533d3b9000f087731fa3a71323c
Summary: GitHub commits: facebook/fbthrift@9b313e2 Reviewed By: bigfootjon fbshipit-source-id: 4398acf93c32d22238246b0df0f01c2605483c97
Summary: GitHub commits: facebook/fbthrift@6fbae55 Reviewed By: bigfootjon fbshipit-source-id: 6f8278f6d8c2f25092b24aa41cee5f32c6e5ac3b
Summary: GitHub commits: facebook/fbthrift@6442718 facebookincubator/fizz@109be99 Reviewed By: bigfootjon fbshipit-source-id: 9c75f0e170a10e2b6cf1913b407f117f1f4091e5
Summary: GitHub commits: facebook/fbthrift@5763f96 Reviewed By: bigfootjon fbshipit-source-id: 56d3adc9df6b8e900d00622b5a7c08fe75d6c6a5
Reviewed By: DenisYaroshevskiy Differential Revision: D44396854 fbshipit-source-id: 17d3f4271f1bf8c403519471a54bd910af38814c
Summary: GitHub commits: facebook/fbthrift@48771f2 Reviewed By: bigfootjon fbshipit-source-id: 9c68f2e95e7b8007941238d42a6714682c5e4d68
Summary: GitHub commits: facebook/fbthrift@53fffa9 facebook/wangle@2ee1978 facebookexperimental/edencommon@deb8089 facebookincubator/fizz@71ea881 facebookincubator/katran@25da03a facebook/mvfst@bf7de7f facebookincubator/velox@651e597 Reviewed By: jailby fbshipit-source-id: f87121525892ad347dcf6d8d9d809828fc737de5
Summary: GitHub commits: facebook/fbthrift@7ab0a87 facebook/proxygen@22104db facebook/watchman@43ed48a facebookincubator/katran@926b507 facebookincubator/velox@3159ba9 Reviewed By: jailby fbshipit-source-id: 25e30991c7cb2da16ff3966627ff5a7fc182a553
Summary: When size-awareness is enabled for mutable workloads, user must call `objCache_->mutateObject(objPtr, mutationCb)` so that object-cache can track the updated size. What should happen inside `mutationCb`? 1. a construction of the new value with deep copy 2. if the new value is going to replace an old value, a destruction of the old value See unit test for examples. ----- Reviewed By: jaesoo-fb Differential Revision: D42872059 fbshipit-source-id: b94eddd6d640a04ee7974f9273509e60d5aa6915
Summary: GitHub commits: facebook/fbthrift@737cfa0 facebook/mvfst@24e04e1 facebookincubator/velox@55daa3e fairinternal/egohowto@f8b1870 Reviewed By: jailby fbshipit-source-id: e801db60548241573e35446c8676039a893752a9
Summary: GitHub commits: facebook/fbthrift@ef080b3 facebook/proxygen@a2d125b Reviewed By: jailby fbshipit-source-id: 1b820a1e3c85011c2b6f535caca237a74ec69568
Summary: GitHub commits: facebook/fbthrift@6b2d7a9 Reviewed By: jailby fbshipit-source-id: 3ccc40b3a1956cde45388e7e0ffad5d6196c517b
Summary: GitHub commits: facebook/fbthrift@639434b facebook/proxygen@a628b5a facebook/watchman@2fc3dba facebookincubator/velox@38d7601 Reviewed By: jailby fbshipit-source-id: 607d6696a1b803a79a4509c55de96e307f3b698a
Summary: GitHub commits: facebook/fbthrift@33a57fc Reviewed By: yns88 fbshipit-source-id: a8cc153a7b8c1491ae264b42e4d1aaf6d637aefe
Summary: GitHub commits: facebook/fb303@28cfb1d facebook/fbthrift@bb45e17 facebook/watchman@7aa562b facebookexperimental/rust-shed@47f504b facebookincubator/velox@58a8e26 Reviewed By: yns88 fbshipit-source-id: e56b09ecacd3bb3a4a8e7a16148faf44d5b0a437
Summary: GitHub commits: facebook/fbthrift@d0d2943 facebook/proxygen@ce6978b facebookincubator/velox@aed1ddf Reviewed By: yns88 fbshipit-source-id: 87cf81b096e7501d13a70ea6062da3e72a39aa65
Summary: GitHub commits: facebook/fbthrift@eb2041c facebook/proxygen@c23cd09 Reviewed By: yns88 fbshipit-source-id: df09b7f4c50e00d5a1491740c999b9008ccfed0e
Summary: GitHub commits: facebook/fbthrift@ea52832 facebook/proxygen@a080dfe facebook/wangle@a153b33 facebookincubator/katran@c24fd07 Reviewed By: yns88 fbshipit-source-id: 9e77bc08070edf5c78c85e856c57defc3b19f454
Summary: GitHub commits: facebook/fbthrift@cd36a53 facebook/watchman@b8c7a3d Reviewed By: yns88 fbshipit-source-id: a1ce2257a8630a9098f239ca49e294cf3dc6bce0
Summary: As title Reviewed By: jaesoo-fb Differential Revision: D44358110 fbshipit-source-id: d9359f25b9f6f58dd7f22cb0b45dd71b6a73848b
Summary: GitHub commits: facebook/fbthrift@59053eb Reviewed By: yns88 fbshipit-source-id: aea2de1893720da1b9b811c1fe00f3ad432fbfc5
Summary: GitHub commits: facebook@f24408d facebook/fb303@f94200e facebook/fbthrift@37243b4 facebook/watchman@c875359 facebookexperimental/rust-shed@2308da6 facebookresearch/vrs@b14c3ee Reviewed By: yns88 fbshipit-source-id: 9fc017f649e948fecee547f0afe1569a43dc03f1
Summary: This should be enabled by default It is an improvement in all cases. It’s roughly the same for hit-path. And, it helps miss path performance since it performs all the checks inline as opposed to thread-hop. Reviewed By: jaesoo-fb Differential Revision: D44554147 fbshipit-source-id: 6ec59d9ac1548ce30199ee1f3c495b92287fb739
Summary: GitHub commits: facebook/fb303@e3d5fe9 facebook/fbthrift@62c3335 facebookexperimental/rust-shed@cf98c92 facebookincubator/velox@0e5889c Reviewed By: yns88 fbshipit-source-id: a4281211b976f5c65929bbc3bd0305225ecabd38
- Switch to isUsingPosixShm() method
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change is