git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 01/11] packed-backend: don't adjust the reference count on lock/unlock

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Sep 8, 2017, 13:51 UTC
Message-ID
<1631f277bb86f653c5c679ca07fbcb2e92410046.1504877858.git.mhagger@alum.mit.edu>
In-Reply-To
<cover.1504877858.git.mhagger@alum.mit.edu>

The old code incremented the packed ref cache reference count when acquiring the packed-refs lock, and decremented the count when releasing the lock. This is unnecessary because:

* Another process cannot change the packed-refs file because it is
  locked.
* When we ourselves change the packed-refs file, we do so by first
  modifying the packed ref-cache, and then writing the data from the
  ref-cache to disk. So the packed ref-cache remains fresh because any
  changes that we plan to make to the file are made in the cache first
  anyway.
So there is no reason for the cache to become stale.

Moreover, the extra reference count causes a problem if we intentionally clear the packed refs cache, as we sometimes need to do if we change the cache in anticipation of writing a change to disk, but then the write to disk fails. In that case, `packed_refs_unlock()` would have no easy way to find the cache whose reference count it needs to decrement.

This whole issue will soon become moot due to upcoming changes that avoid changing the in-memory cache as part of updating the packed-refs on disk, but this change makes that transition easier.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs/packed-backend.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 412c85034f..b76f14e5b3 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -525,7 +525,6 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err)
 				"packed_refs_lock");
 	static int timeout_configured = 0;
 	static int timeout_value = 1000;
-	struct packed_ref_cache *packed_ref_cache;
 
 	if (!timeout_configured) {
 		git_config_get_int("core.packedrefstimeout", &timeout_value);
@@ -560,9 +559,11 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err)
 	 */
 	validate_packed_ref_cache(refs);
 
-	packed_ref_cache = get_packed_ref_cache(refs);
-	/* Increment the reference count to prevent it from being freed: */
-	acquire_packed_ref_cache(packed_ref_cache);
+	/*
+	 * Now make sure that the packed-refs file as it exists in the
+	 * locked state is loaded into the cache:
+	 */
+	get_packed_ref_cache(refs);
 	return 0;
 }
 
@@ -576,7 +577,6 @@ void packed_refs_unlock(struct ref_store *ref_store)
 	if (!is_lock_file_locked(&refs->lock))
 		die("BUG: packed_refs_unlock() called when not locked");
 	rollback_lock_file(&refs->lock);
-	release_packed_ref_cache(refs->cache);
 }
 
 int packed_refs_is_locked(struct ref_store *ref_store)
-- 
2.14.1
Previous: Michael HaggertyNext: Michael Haggerty
Message 2 of 15 in “Implement transactions for the packed ref store”
  1. 00/11 Implement transactions for the packed ref storeMichael Haggerty, Sep 8, 2017
  2. 01/11 packed-backend: don't adjust the reference count on lock/unlockMichael Haggerty, Sep 8, 2017
  3. 02/11 struct ref_transaction: add a place for backends to store dataMichael Haggerty, Sep 8, 2017
  4. 05/11 files_pack_refs(): use a reference transaction to write packed refsMichael Haggerty, Sep 8, 2017
  5. 03/11 packed_ref_store: implement reference transactionsMichael Haggerty, Sep 8, 2017
  6. 07/11 files_initial_transaction_commit(): use a transaction for packed refsMichael Haggerty, Sep 8, 2017
  7. 10/11 packed-backend: rip out some now-unused codeMichael Haggerty, Sep 8, 2017
  8. 11/11 files_transaction_finish(): delete reflogs before referencesMichael Haggerty, Sep 8, 2017
  9. 04/11 packed_delete_refs(): implement methodMichael Haggerty, Sep 8, 2017
  10. 09/11 files_ref_store: use a transaction to update packed refsMichael Haggerty, Sep 8, 2017
  11. 06/11 prune_refs(): also free the linked listMichael Haggerty, Sep 8, 2017
  12. 08/11 t1404: demonstrate two problems with reference transactionsMichael Haggerty, Sep 8, 2017
  13. Jeff KingSep 9, 2017
  14. Michael HaggertySep 10, 2017
  15. Jeff KingSep 9, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.