Implement Internal Cache

doc/decisions/0_drafts/operation_sequences.md

@@ -218,15 +218,15 @@ As can be seen, the change tracking within the `dbus` and `logchange` plugins wr
    This alternative would put most of the burden onto the plugin authors.
    Depending on what the plugins do, every plugin may also need to deep-dup every keyset of every parent it ever receives via `kdbGet`.
    This will increase memory usage.
-   However, this could be paired with the [COW semantics](../4_decided/internal_cache.md) so the memory toll would not be that big of a deal.
+   However, this could be paired with the [COW semantics](../6_implemented/internal_cache.md) so the memory toll would not be that big of a deal.
    The biggest problem with this approach would be the unnecessary duplication of the non-trivial change tracking algorithm.
 
 6. Don't restrict sequences further and provide a common framework to handle change tracking correctly.
 
    As the problem has only been observed with plugins doing their own change tracking, we could provide a general change tracking framework within Elektra.
    This way, we have only one such algorithm in a central place, and plugin authors don't have to think about the sequences their plugins are called by developers.
 
-   This approach can also be paired with [COW semantics](../4_decided/internal_cache.md), so that memory toll will be kept low.
+   This approach can also be paired with [COW semantics](../6_implemented/internal_cache.md), so that memory toll will be kept low.
    A separate [decision for change tracking](../3_in_review/change_tracking.md) is currently in progress.
 
    Should we observe this problem with use cases other than change tracking, we can provide general frameworks for those too.
@@ -244,7 +244,7 @@ As can be seen, the change tracking within the `dbus` and `logchange` plugins wr
 ## Related Decisions
 
 - [Change Tracking](../3_in_review/change_tracking.md)
-- [Internal KeySet Cache](../4_decided/internal_cache.md)
+- [Internal KeySet Cache](../6_implemented/internal_cache.md)
 - []()
 
 ## Notes

doc/decisions/0_drafts/transformations.md

@@ -179,7 +179,7 @@ A drawback to this solution is that it adds some complexity to `libelektra-core`
 - [Hooks](../5_partially_implemented/hooks.md)
 - [Change Tracking](../3_in_review/change_tracking.md)
 - [Operation Sequences](../0_drafts/operation_sequences.md)
-- [Internal Cache](../4_decided/internal_cache.md)
+- [Internal Cache](../6_implemented/internal_cache.md)
 
 ## Notes
 

doc/decisions/0a_postponed/global_validation.md

@@ -38,7 +38,7 @@ For now, the specification can only refer to what applications request by `kdbGe
 
 ## Related Decisions
 
-- [Internal Cache](../4_decided/internal_cache.md)
+- [Internal Cache](../6_implemented/internal_cache.md)
 
 ## Notes
 

doc/decisions/3_in_review/change_tracking.md

@@ -81,7 +81,7 @@ A problem with this approach is that the internally stored keys are recreated as
 ### Combine with internal cache
 
 We already decided that we want to have an internal deep-duped keyset of all the keys we returned.
-See [internal cache decision](../4_decided/internal_cache.md).
+See [internal cache decision](../6_implemented/internal_cache.md).
 
 The difference to `backendData->keys` is that this cache is not recreated each time `kdbGet` is called.
 
@@ -175,7 +175,7 @@ The changetracking plugin needs to export at least functions for the following t
 ## Decision
 
 Plugin and application developers can declare that changetracking is required via a contract.
-Store deep-duped copy-on-write returned keys in a separate keyset, which we might also use as [internal cache](../4_decided/internal_cache.md).
+Store deep-duped copy-on-write returned keys in a separate keyset, which we might also use as [internal cache](../6_implemented/internal_cache.md).
 The whole changetracking logic lives within `libelektra-kdb`.
 We provide an API for developers with `libelektra-kdb`.
 

doc/decisions/6_implemented/copy_on_write.md

@@ -8,7 +8,7 @@ Most of those copied keys are never modified, but are required to be detached fr
 We want to introduce an in-memory copy-on-write mechanism to lower our memory usage.
 
 In the near future, Elektra will also gain facilities for [change tracking](../3_in_review/change_tracking.md) and session recording, both of which will potentially again duplicate keys.
-There are also aspirations to create a new, simple [internal cache](../4_decided/internal_cache.md) that would also benefit from a copy-on-write mechanism.
+There are also aspirations to create a new, simple [internal cache](internal_cache.md) that would also benefit from a copy-on-write mechanism.
 
 ## Constraints
 
@@ -526,6 +526,6 @@ Implement the full-blown COW approach.
 ## Related Decisions
 
 - [change tracking](../3_in_review/change_tracking.md)
-- [internal cache](../4_decided/internal_cache.md)
+- [internal cache](internal_cache.md)
 
 ## Notes

doc/decisions/4_decided/internal_cache.md → doc/decisions/6_implemented/internal_cache.md

@@ -201,7 +201,7 @@ I.e. the original keyset is not changed.
 The name is not relevant.
 It is always read-only, because the key is in at least one keyset (the internal one).
 
-Possible copy-on-write implementations are described in [another decision](../6_implemented/copy_on_write.md).
+Possible copy-on-write implementations are described in [another decision](copy_on_write.md).
 
 ## Decision
 
@@ -214,7 +214,7 @@ If the user tries to change the value or metadata of these keys, the data gets d
 I.e. the data of the original keys is not changed.
 The name is not relevant.
 It is always read-only, because the key is in at least one keyset (the internal one).
-Possible copy-on-write implementations are described in [another decision](../6_implemented/copy_on_write.md).
+Possible copy-on-write implementations are described in [another decision](copy_on_write.md).
 
 In `kdbSet` we use the user-provided `KeySet` for all backends strictly below `parentKey` as before.
 For the backend that contains `parentKey`, we start with the internally cached data.
@@ -229,14 +229,14 @@ The copy-on-write solution also does not require any changes or restrictions to
 
 ## Implications
 
-- Before we can implement this decision, we need to implement the [copy-on-write decision](../6_implemented/copy_on_write.md).
+- Before we can implement this decision, we need to implement the [copy-on-write decision](copy_on_write.md).
 - `kdbGet` will only return copy-on-write copies of keys below `parentKey` from the internal cache.
 - `kdbSet` will use the keys within the internal cache to supplement all the keys above `parentKey` so that backends can write the correct data.
 
 ## Related Decisions
 
 - [Global Validation](../0a_postponed/global_validation.md)
-- [Copy On Write](../6_implemented/copy_on_write.md)
+- [Copy On Write](copy_on_write.md)
 
 ## Notes
 

doc/news/2023-03-03_0.9.12.md

@@ -361,7 +361,7 @@ This section keeps you up-to-date with the multi-language support provided by El
 - Revive [keyname decision](../decisions/4_decided/keyname.md). _(@kodebach)_
 - Add decisions for [constructor functions](../decisions/0_drafts/constructor_functions.md) and [builder functions](../decisions/0_drafts/builder_functions.md). _(@kodebach)_
 - Add decision for [copy-on-write](../decisions/6_implemented/copy_on_write.md) and provide implementation suggestions. _(Maximilian Irlinger @atmaxinger)_
-- Update [internal cache](../decisions/4_decided/internal_cache.md). _(Markus Raab)_ _(@kodebach)_
+- Update [internal cache](../decisions/6_implemented/internal_cache.md). _(Markus Raab)_ _(@kodebach)_
 - Create [transformations](../decisions/0_drafts/transformations.md). _(Maximilian Irlinger @atmaxinger)_
 - Replace TOC-style [README.md](../decisions/README.md) with folders and generate HTML for website. _(@kodebach)_
 - Restructured decisions directories based on new agreed-upon [steps](../decisions/STEPS.md). _(Maximilian Irlinger @atmaxinger)_

doc/news/_preparation_next_release.md

@@ -190,9 +190,9 @@ The text below summarizes updates to the [C (and C++)-based libraries](https://w
 - <<TODO>>
 - <<TODO>>
 
-### <<Library>>
+### kdb
 
-- <<TODO>>
+- Implement internal cache as described in [the decision](/doc/decisions/6_implemented/internal_cache.md). _(Maximilian Irlinger @atmaxinger)_
 - <<TODO>>
 - Add new changetracking API _(Maximilian Irlinger @atmaxinger)_
 - Fix unwanted removal of subkeys when using mv _(Hannes Laimer @hannes99)_

src/libs/elektra/kdb.c

@@ -1844,6 +1844,8 @@ int kdbGet (KDB * handle, KeySet * ks, Key * parentKey)
 		return -1;
 	}
 
+	KeySet * fromInternalCache = ksBelow (handle->allKeys, initialParent);
+
 	parentKey->hasReadOnlyName = false;
 	parentKey->hasReadOnlyValue = false;
 
@@ -1856,19 +1858,33 @@ int kdbGet (KDB * handle, KeySet * ks, Key * parentKey)
 	// Step 5: remove up-to-date backends
 	for (elektraCursor i = 0; i < ksGetSize (backends); i++)
 	{
-		if (keyGetMeta (ksAtCursor (backends, i), "meta:/internal/kdbneedsupdate") == NULL)
+		Key * backendKey = ksAtCursor (backends, i);
+		if (keyGetMeta (backendKey, "meta:/internal/kdbneedsupdate") == NULL)
 		{
 			elektraKsPopAtCursor (backends, i);
 			--i;
 		}
+		else if (fromInternalCache != NULL)
+		{
+			// Remove keys from fromInternalCache that have updated data in backends
+			ksDel (ksCut (fromInternalCache, backendKey));
+		}
+	}
+
+	if (fromInternalCache != NULL)
+	{
+		// Create a deep-duplication of the cache keys
+		KeySet * t = fromInternalCache;
+		fromInternalCache = ksDeepDup (fromInternalCache);
+		ksDel (t);
 	}
 
 	bool procOnly = keyGetNamespace (ksAtCursor (backends, 0)) == KEY_NS_PROC &&
 			keyGetNamespace (ksAtCursor (backends, ksGetSize (backends) - 1)) == KEY_NS_PROC;
 
 	// Step 6: return if no backends left
 	// HACK: for gopts
-	if (ksGetSize (backends) == 0 || (goptsActive && procOnly && ksGetSize (backends) == 1))
+	if (ksGetSize (fromInternalCache) == 0 && (ksGetSize (backends) == 0 || (goptsActive && procOnly && ksGetSize (backends) == 1)))
 	{
 		keyCopy (parentKey, initialParent, KEY_CP_NAME | KEY_CP_VALUE);
 		keyDel (initialParent);
@@ -1878,6 +1894,7 @@ int kdbGet (KDB * handle, KeySet * ks, Key * parentKey)
 
 		ksDel (backends);
 		ksDel (allBackends);
+		ksDel (fromInternalCache);
 		errno = errnosave;
 		return 2;
 	}
@@ -1961,13 +1978,22 @@ int kdbGet (KDB * handle, KeySet * ks, Key * parentKey)
 	// Step 14: run spec plugin
 	parentKey->hasReadOnlyName = true;
 	parentKey->hasReadOnlyValue = true;
-	if (handle->hooks.spec.plugin && handle->hooks.spec.copy (handle->hooks.spec.plugin, dataKs, parentKey, true) == -1)
+
+	KeySet * keysForSpec = ksNew (ksGetSize (dataKs) + ksGetSize (fromInternalCache), KS_END);
+	ksAppend (keysForSpec, dataKs);
+	ksAppend (keysForSpec, fromInternalCache);
+
+	if (handle->hooks.spec.plugin && handle->hooks.spec.copy (handle->hooks.spec.plugin, keysForSpec, parentKey, true) == -1)
 	{
 		parentKey->hasReadOnlyName = false;
 		parentKey->hasReadOnlyValue = false;
 		ksDel (dataKs);
+		ksDel (keysForSpec);
 		goto error;
 	}
+
+	ksDel (keysForSpec);
+
 	parentKey->hasReadOnlyName = false;
 	parentKey->hasReadOnlyValue = false;
 
@@ -2007,6 +2033,8 @@ int kdbGet (KDB * handle, KeySet * ks, Key * parentKey)
 	// Step 18: merge data into ks and return
 	ksClear (dataKs);
 	backendsMerge (backends, dataKs);
+
+	ksAppend (ks, fromInternalCache);
 	ksAppend (ks, dataKs);
 
 	// TODO (atmaxinger): should we have a default:/ backend?
@@ -2041,6 +2069,7 @@ int kdbGet (KDB * handle, KeySet * ks, Key * parentKey)
 		ksAppendDup (handle->allKeys, dataKs);
 	}
 	ksDel (dataKs);
+	ksDel (fromInternalCache);
 
 	errno = errnosave;
 	return procOnly ? 2 : 1;
@@ -2055,6 +2084,7 @@ int kdbGet (KDB * handle, KeySet * ks, Key * parentKey)
 
 	ksDel (backends);
 	ksDel (allBackends);
+	if (fromInternalCache) ksDel (fromInternalCache);
 
 	errno = errnosave;
 	return -1;
@@ -2474,7 +2504,24 @@ int kdbSet (KDB * handle, KeySet * ks, Key * parentKey)
 	{
 		Key * backendKey = ksAtCursor (backends, i);
 
-		KeySet * below = ksBelow (ks, backendKey);
+		KeySet * below = NULL;
+		if (handle->allKeys != NULL)
+		{
+			below = ksBelow (handle->allKeys, backendKey);
+			ksDel (ksCut (below, initialParent));
+
+			KeySet * bBackend = ksBelow (ks, backendKey);
+			KeySet * bParent = ksBelow (bBackend, initialParent);
+			ksAppend (below, bParent);
+
+			ksDel (bParent);
+			ksDel (bBackend);
+		}
+		else
+		{
+			below = ksBelow (ks, backendKey);
+		}
+
 		KeySet * deepDupedBelow = ksDeepDup (below);
 		ksAppend (setKs, deepDupedBelow);
 		ksDel (below);

tests/ctest/CMakeLists.txt

@@ -45,6 +45,8 @@ target_link_elektra (test_diffprivate elektra-kdb)
 target_link_elektra (test_error elektra-kdb)
 target_link_elektra (test_record elektra-kdb elektra-record)
 
+target_link_elektra (test_internalcache elektra-kdb)
+
 if (TARGET elektra-notification
     AND TARGET elektra-dbus
     AND TARGET elektra-internalnotification

tests/ctest/test_internalcache.c

@@ -0,0 +1,97 @@
+/**
+ * @file
+ *
+ * @brief Test suite for internal data structures.
+ *
+ * @copyright BSD License (see LICENSE.md or https://www.libelektra.org)
+ */
+
+#include "kdbhelper.h"
+#include "kdbprivate.h"
+#include <tests_internal.h>
+
+static void test_doubleGet (void)
+{
+	printf ("running %s\n", __func__);
+
+	// Setup
+	Key * parentKey = keyNew ("/somewhere", KS_END);
+	KeySet * ks = ksNew (0, KS_END);
+	KDB * kdb = kdbOpen (ksNew (0, KS_END), parentKey);
+	kdbGet (kdb, ks, parentKey);
+	ksAppendKey (ks, keyNew ("user:/somewhere", KEY_VALUE, "abc", KEY_END));
+	ksAppendKey (ks, keyNew ("user:/somewhere/key", KEY_VALUE, "xyz", KEY_END));
+	kdbSet (kdb, ks, parentKey);
+	kdbClose (kdb, parentKey);
+
+	// Scenario
+	kdb = kdbOpen (ksNew (0, KS_END), parentKey);
+
+	KeySet * ks1 = ksNew (0, KS_END);
+	KeySet * ks2 = ksNew (0, KS_END);
+
+	kdbGet (kdb, ks1, parentKey);
+	succeed_if (ksLookupByName (ks1, "/somewhere/key", 0) != NULL, "should find key (1)");
+	kdbGet (kdb, ks2, parentKey);
+	succeed_if (ksLookupByName (ks2, "/somewhere/key", 0) != NULL, "should find key (2)");
+
+	kdbClose (kdb, parentKey);
+
+	ksDel (ks);
+	ksDel (ks1);
+	ksDel (ks2);
+	keyDel (parentKey);
+}
+
+static void test_get_modified_keys (void)
+{
+	printf ("running %s\n", __func__);
+
+	// Setup
+	Key * parentKey = keyNew ("/somewhere", KS_END);
+	KeySet * ksInitial = ksNew (0, KS_END);
+	KDB * kdb = kdbOpen (ksNew (0, KS_END), parentKey);
+	kdbGet (kdb, ksInitial, parentKey);
+	ksAppendKey (ksInitial, keyNew ("user:/somewhere", KEY_VALUE, "abc", KEY_END));
+	ksAppendKey (ksInitial, keyNew ("user:/somewhere/key", KEY_VALUE, "xyz", KEY_END));
+	kdbSet (kdb, ksInitial, parentKey);
+	kdbClose (kdb, parentKey);
+
+	// Scenario
+	kdb = kdbOpen (ksNew (0, KS_END), parentKey);
+
+	KeySet * ks1 = ksNew (0, KS_END);
+	KeySet * ks2 = ksNew (0, KS_END);
+
+	kdbGet (kdb, ks1, parentKey);
+	succeed_if_keyset_contains_key_with_string (ks1, "/somewhere/key", "xyz");
+	ksAppendKey (ks1, keyNew ("user:/somewhere/key", KEY_VALUE, "updated", KEY_END));
+	ksAppendKey (ks1, keyNew ("user:/somewhere/newkey", KEY_VALUE, "new", KEY_END));
+	kdbSet (kdb, ks1, parentKey);
+
+	kdbGet (kdb, ks2, parentKey);
+	succeed_if_keyset_contains_key_with_string (ks2, "/somewhere/key", "updated");
+	succeed_if_keyset_contains_key_with_string (ks2, "/somewhere/newkey", "new");
+
+	kdbClose (kdb, parentKey);
+
+	ksDel (ksInitial);
+	ksDel (ks1);
+	ksDel (ks2);
+	keyDel (parentKey);
+}
+
+int main (int argc, char ** argv)
+{
+	printf ("INTERNAL CACHE TESTS\n");
+	printf ("====================\n\n");
+
+	init (argc, argv);
+
+	test_doubleGet ();
+	test_get_modified_keys ();
+
+	printf ("\ntest_internalcache RESULTS: %d test(s) done. %d error(s).\n", nbTest, nbError);
+
+	return nbError;
+}