git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Stefan Beller" <sbeller@google.com>, "Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Brandon Williams" <bmwill@google.com>,
	git@vger.kernel.org, "Michael Haggerty" <mhagger@alum.mit.edu>
Subject: [PATCH 01/10] packed-backend: don't adjust the reference count on lock/unlock
Date: Tue, 29 Aug 2017 10:20:25 +0200	[thread overview]
Message-ID: <c5d66cadb70efdb5b5ce91fbcc5673621f5d4792.1503993268.git.mhagger@alum.mit.edu> (raw)
In-Reply-To: <cover.1503993268.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 a locked packed-refs
file cannot be changed, 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 | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 412c85034f..7e348feac3 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,7 @@ 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);
+	get_packed_ref_cache(refs);
 	return 0;
 }
 
@@ -576,7 +573,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


  reply	other threads:[~2017-08-29  8:21 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-29  8:20 [PATCH 00/10] Implement transactions for the packed ref store Michael Haggerty
2017-08-29  8:20 ` Michael Haggerty [this message]
2017-09-08  6:52   ` [PATCH 01/10] packed-backend: don't adjust the reference count on lock/unlock Jeff King
2017-09-08 10:02     ` Michael Haggerty
2017-08-29  8:20 ` [PATCH 02/10] struct ref_transaction: add a place for backends to store data Michael Haggerty
2017-09-08  7:02   ` Jeff King
2017-09-08  8:19     ` Michael Haggerty
2017-09-08  8:33       ` Jeff King
2017-08-29  8:20 ` [PATCH 03/10] packed_ref_store: implement reference transactions Michael Haggerty
2017-08-29  8:20 ` [PATCH 04/10] packed_delete_refs(): implement method Michael Haggerty
2017-08-29 18:07   ` Brandon Williams
2017-08-30  3:00     ` Michael Haggerty
2017-08-29  8:20 ` [PATCH 05/10] files_pack_refs(): use a reference transaction to write packed refs Michael Haggerty
2017-08-29  8:20 ` [PATCH 06/10] files_initial_transaction_commit(): use a transaction for " Michael Haggerty
2017-09-08  7:27   ` Jeff King
2017-09-08 10:04     ` Michael Haggerty
2017-08-29  8:20 ` [PATCH 07/10] t1404: demonstrate two problems with reference transactions Michael Haggerty
2017-08-30 17:21   ` Stefan Beller
2017-08-31  3:42     ` Michael Haggerty
2017-09-08  4:44   ` Junio C Hamano
2017-09-08  7:45     ` Jeff King
2017-09-08 10:06     ` Michael Haggerty
2017-08-29  8:20 ` [PATCH 08/10] files_ref_store: use a transaction to update packed refs Michael Haggerty
2017-09-08  7:38   ` Jeff King
2017-09-08 12:44     ` Michael Haggerty
2017-09-08 12:57       ` Jeff King
2017-08-29  8:20 ` [PATCH 09/10] packed-backend: rip out some now-unused code Michael Haggerty
2017-08-29 18:24   ` Brandon Williams
2017-08-29  8:20 ` [PATCH 10/10] files_transaction_finish(): delete reflogs before references Michael Haggerty
2017-08-29 18:30 ` [PATCH 00/10] Implement transactions for the packed ref store Brandon Williams
2017-09-08  7:42 ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c5d66cadb70efdb5b5ce91fbcc5673621f5d4792.1503993268.git.mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=avarab@gmail.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).