Skip to content
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

Add a separate property, isCacheable, to SharedKeys. #195

Merged
merged 2 commits into from
Dec 11, 2023

Conversation

jianminzhao
Copy link
Contributor

If cacheable, Dict::get(key &keyToFind) will cache the string key to integer key and enhance the performance of the ensuing gets. This is good if the doucment is not changed.

For PersistentSharedKey, the library manages the property: it is cacheable only if it is not in a transaction when Dict::get is called.

For SharedKeys, the client manages it. If it anticipates the sharedKeys may outlive the underlying document changes, it should call SharedKeys::disableCaching().

If cacheable, Dict::get(key &keyToFind) will cache the string key to integer key and enhance the performance of the ensuing gets. This is good if the doucment is not changed.

For PersistentSharedKey, the library manages the property: it is cacheable only if it is not in a transaction when Dict::get is called.

For SharedKeys, the client manages it. If it anticipates the sharedKeys may outlive the underlying document changes, it should call SharedKeys::disableCaching().
Copy link
Contributor

@snej snej left a comment

Choose a reason for hiding this comment

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

LITECORE_CPPTEST isn't an accurate name because this isn't the LiteCore project. How about FLEECE_TEST?

@jianminzhao
Copy link
Contributor Author

The reason of LITECORE_CPPTEST is because the automatic tests are run from LiteCore. The cmake project of CppTests lumps all tests from fleece with the LiteCore's tests.

@jianminzhao jianminzhao requested a review from snej December 4, 2023 23:35
@snej
Copy link
Contributor

snej commented Dec 7, 2023

I would rather not have stuff in Fleece that's so directly tied to LiteCore. The test stuff in LiteCore could define FLEECE_TEST as well.

@snej
Copy link
Contributor

snej commented Dec 7, 2023

Or you could get around the need for a special macro by adding a public method to access _hasNumericKey, called something like isShared.

@@ -262,7 +262,7 @@ namespace fleece {

caughtException = false;
// ++iter will throw if already at the end.
// OutOfRange exception should contain the backtrace.
// OutOfRange exception should not contain the backtrace.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a mistake. Should be "... not contain the backtrace."

@jianminzhao jianminzhao merged commit edfe2db into master Dec 11, 2023
3 checks passed
@jianminzhao jianminzhao deleted the cbl-4547-fleece branch December 11, 2023 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants