[PATCH] Make the global packed_git variable static to sha1_file.c.
This is a first step in making the codebase thread-safe. By and
large, the operations which might benefit from threading are those
that work with pack files (e.g., checkout, blame), so the focus of
this patch is stop leaking the global list of pack files outside of
sha1_file.c.
The next step will be to control access to the list of pack files
with a mutex. However, that alone is not enough to make pack file
access thread safe. Even in a read-only operation, the window list
associated with each pack file will need to be controlled.
Additionally, the global counters in sha1_file.c will need to be
controlled.
This patch is a pure refactor with no functional changes, so it
shouldn't require any additional tests. Adding the actual locks
will be a functional change, and will require additional tests.
Signed-off-by: Stefan Zager sza...@chromium.org
---
builtin/count-objects.c | 44 ++-
builtin/fsck.c | 46 +++-
builtin/gc.c | 26 +++
builtin/pack-objects.c | 189 ---
builtin/pack-redundant.c | 37 +++---
cache.h | 18 -
fast-import.c| 4 +-
http-backend.c | 28 ---
http-push.c | 4 +-
http-walker.c| 2 +-
pack-revindex.c | 20 ++---
server-info.c| 36 -
sha1_file.c | 52 ++---
sha1_name.c | 18 -
14 files changed, 327 insertions(+), 197 deletions(-)
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a7f70cb..a27c006 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -83,14 +83,32 @@ static char const * const count_objects_usage[] = {
NULL
};
+struct pack_data {
+ unsigned long packed;
+ off_t size_pack;
+ unsigned long num_pack;
+};
+
+static int count_pack_objects(struct packed_git *p, void *data)
+{
+ struct pack_data *pd = (struct pack_data *) data;
+ if (p-pack_local !open_pack_index(p)) {
+ pd-packed += p-num_objects;
+ pd-size_pack += p-pack_size + p-index_size;
+ pd-num_pack++;
+ }
+ return 0;
+}
+
int cmd_count_objects(int argc, const char **argv, const char *prefix)
{
int i, verbose = 0, human_readable = 0;
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
- unsigned long loose = 0, packed = 0, packed_loose = 0;
+ unsigned long loose = 0, packed_loose = 0;
off_t loose_size = 0;
+ struct pack_data pd = {0, 0, 0};
struct option opts[] = {
OPT__VERBOSE(verbose, N_(be verbose)),
OPT_BOOL('H', human-readable, human_readable,
@@ -118,41 +136,29 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
closedir(d);
}
if (verbose) {
- struct packed_git *p;
- unsigned long num_pack = 0;
- off_t size_pack = 0;
struct strbuf loose_buf = STRBUF_INIT;
struct strbuf pack_buf = STRBUF_INIT;
struct strbuf garbage_buf = STRBUF_INIT;
- if (!packed_git)
- prepare_packed_git();
- for (p = packed_git; p; p = p-next) {
- if (!p-pack_local)
- continue;
- if (open_pack_index(p))
- continue;
- packed += p-num_objects;
- size_pack += p-pack_size + p-index_size;
- num_pack++;
- }
+ prepare_packed_git();
+ foreach_packed_git(count_pack_objects, NULL, pd);
if (human_readable) {
strbuf_humanise_bytes(loose_buf, loose_size);
- strbuf_humanise_bytes(pack_buf, size_pack);
+ strbuf_humanise_bytes(pack_buf, pd.size_pack);
strbuf_humanise_bytes(garbage_buf, size_garbage);
} else {
strbuf_addf(loose_buf, %lu,
(unsigned long)(loose_size / 1024));
strbuf_addf(pack_buf, %lu,
- (unsigned long)(size_pack / 1024));
+ (unsigned long)(pd.size_pack / 1024));
strbuf_addf(garbage_buf, %lu,
(unsigned long)(size_garbage / 1024));
}
printf(count: %lu\n, loose);
printf(size: %s\n, loose_buf.buf);
- printf(in-pack: %lu\n, packed);
- printf(packs: %lu\n, num_pack);
+ printf(in-pack: %lu\n, pd.packed);
+ printf(packs: %lu\n, pd.num_pack);
printf(size-pack: %s\n, pack_buf.buf);
Re: [PATCH] Make the global packed_git variable static to sha1_file.c.
On Tue, Feb 11, 2014 at 11:29 PM, Chris Packham judge.pack...@gmail.com wrote: Hi, On 12/02/14 14:57, Stefan Zager wrote: From b4796d9d99c03b0b7cddd50808a41413e45f1129 Mon Sep 17 00:00:00 2001 From: Stefan Zager sza...@chromium.org Date: Mon, 10 Feb 2014 16:55:12 -0800 Subject: [PATCH] Make the global packed_git variable static to sha1_file.c. I'm not really qualified to comment on substance but there are some basic style issues w.r.t. whitespace namely using 4 spaces for indent and mixing tabs/spaces. This might seem pedantic for the first round of a patch but it does put off reviewers. From Documentation/CodingGuidelines: - We use tabs to indent, and interpret tabs as taking up to 8 spaces. My bad, I will upload a fixed patch. In my defense: I edited the code in emacs and then ran M-x tabify over the entire file. But that had the unfortunate side effect of adding a bunch of whitespace-only changes to the diff, illuminating the fact that there is already mixed whitespace in the existing code. So I had to go back and selectively tabify my changes, and I clearly missed a bunch. If anyone has a recommendation for a less labor-intensive way to do this in emacs, I'd be very grateful. Thanks, Stefan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Make the global packed_git variable static to sha1_file.c.
Stefan Zager sza...@google.com writes: On Tue, Feb 11, 2014 at 11:29 PM, Chris Packham judge.pack...@gmail.com wrote: Hi, On 12/02/14 14:57, Stefan Zager wrote: From b4796d9d99c03b0b7cddd50808a41413e45f1129 Mon Sep 17 00:00:00 2001 From: Stefan Zager sza...@chromium.org Date: Mon, 10 Feb 2014 16:55:12 -0800 Subject: [PATCH] Make the global packed_git variable static to sha1_file.c. I'm not really qualified to comment on substance but there are some basic style issues w.r.t. whitespace namely using 4 spaces for indent and mixing tabs/spaces. This might seem pedantic for the first round of a patch but it does put off reviewers. From Documentation/CodingGuidelines: - We use tabs to indent, and interpret tabs as taking up to 8 spaces. My bad, I will upload a fixed patch. In my defense: I edited the code in emacs and then ran M-x tabify over the entire file. But that had the unfortunate side effect of adding a bunch of whitespace-only changes to the diff, illuminating the fact that there is already mixed whitespace in the existing code. So I had to go back and selectively tabify my changes, and I clearly missed a bunch. If anyone has a recommendation for a less labor-intensive way to do this in emacs, I'd be very grateful. C-c . RET linux RET before entering any changes. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Make the global packed_git variable static to sha1_file.c.
Stefan Zager sza...@google.com writes:
If anyone has a recommendation for a less labor-intensive way to do
this in emacs, I'd be very grateful.
This is not do this in emacs, but here is a possible approach.
You can ask git diff about what you changed, and actually apply
the change while fixing whitespace errors. I.e.
git diff sha1_file.c | git apply --cached --whitespace=fix
git diff
git checkout sha1_file.c
The first step will add a cleaned-up version to your index.
The second diff (optional) is to see what whitespace errors are
introduced when going from that cleaned-up version to what you have
in the working tree.
With the last step you would update the working tree version to the
cleaned-up version from the index.
[alias]
wsadd = !sh -c 'git diff -- \$@\ | git apply --cached
--whitespace=fix;\
git co -- ${1-.} \$@\' -
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Make the global packed_git variable static to sha1_file.c.
From b4796d9d99c03b0b7cddd50808a41413e45f1129 Mon Sep 17 00:00:00 2001
From: Stefan Zager sza...@chromium.org
Date: Mon, 10 Feb 2014 16:55:12 -0800
Subject: [PATCH] Make the global packed_git variable static to sha1_file.c.
This is a first step in making the codebase thread-safe. By and
large, the operations which might benefit from threading are those
that work with pack files (e.g., checkout, blame), so the focus of
this patch is stop leaking the global list of pack files outside of
sha1_file.c.
The next step will be to control access to the list of pack files
with a mutex. However, that alone is not enough to make pack file
access thread safe. Even in a read-only operation, the window list
associated with each pack file will need to be controlled.
Additionally, the global counters in sha1_file.c will need to be
controlled.
This patch is a pure refactor with no functional changes, so it
shouldn't require any additional tests. Adding the actual locks
will be a functional change, and will require additional tests.
Signed-off-by: Stefan Zager sza...@chromium.org
---
builtin/count-objects.c | 44 ++-
builtin/fsck.c | 46 +++-
builtin/gc.c | 26 +++
builtin/pack-objects.c | 188 ---
builtin/pack-redundant.c | 37 +++---
cache.h | 16 +++-
fast-import.c| 4 +-
http-backend.c | 28 ---
http-push.c | 4 +-
http-walker.c| 2 +-
pack-revindex.c | 20 ++---
server-info.c| 35 +
sha1_file.c | 35 -
sha1_name.c | 18 -
14 files changed, 315 insertions(+), 188 deletions(-)
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a7f70cb..5a8e487 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -83,14 +83,32 @@ static char const * const count_objects_usage[] = {
NULL
};
+struct pack_data {
+unsigned long packed;
+off_t size_pack;
+unsigned long num_pack;
+};
+
+int pack_data_fn(struct packed_git *p, void *data)
+{
+struct pack_data *pd = (struct pack_data *) data;
+if (p-pack_local !open_pack_index(p)) {
+ pd-packed += p-num_objects;
+ pd-size_pack += p-pack_size + p-index_size;
+ pd-num_pack++;
+}
+return 1;
+}
+
int cmd_count_objects(int argc, const char **argv, const char *prefix)
{
int i, verbose = 0, human_readable = 0;
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
- unsigned long loose = 0, packed = 0, packed_loose = 0;
+ unsigned long loose = 0, packed_loose = 0;
off_t loose_size = 0;
+struct pack_data pd = {0,0,0};
struct option opts[] = {
OPT__VERBOSE(verbose, N_(be verbose)),
OPT_BOOL('H', human-readable, human_readable,
@@ -118,41 +136,29 @@ int cmd_count_objects(int argc, const char **argv, const
char *prefix)
closedir(d);
}
if (verbose) {
- struct packed_git *p;
- unsigned long num_pack = 0;
- off_t size_pack = 0;
struct strbuf loose_buf = STRBUF_INIT;
struct strbuf pack_buf = STRBUF_INIT;
struct strbuf garbage_buf = STRBUF_INIT;
- if (!packed_git)
- prepare_packed_git();
- for (p = packed_git; p; p = p-next) {
- if (!p-pack_local)
- continue;
- if (open_pack_index(p))
- continue;
- packed += p-num_objects;
- size_pack += p-pack_size + p-index_size;
- num_pack++;
- }
+ prepare_packed_git();
+ foreach_packed_git(pack_data_fn, NULL, pd);
if (human_readable) {
strbuf_humanise_bytes(loose_buf, loose_size);
- strbuf_humanise_bytes(pack_buf, size_pack);
+ strbuf_humanise_bytes(pack_buf, pd.size_pack);
strbuf_humanise_bytes(garbage_buf, size_garbage);
} else {
strbuf_addf(loose_buf, %lu,
(unsigned long)(loose_size / 1024));
strbuf_addf(pack_buf, %lu,
- (unsigned long)(size_pack / 1024));
+ (unsigned long)(pd.size_pack / 1024));
strbuf_addf(garbage_buf, %lu,
(unsigned long)(size_garbage / 1024));
}
printf(count: %lu\n, loose);
printf(size: %s\n, loose_buf.buf);
- printf(in-pack: %lu\n, packed);
- printf(packs: %lu
Re: [PATCH] Make the global packed_git variable static to sha1_file.c.
Hi, On 12/02/14 14:57, Stefan Zager wrote: From b4796d9d99c03b0b7cddd50808a41413e45f1129 Mon Sep 17 00:00:00 2001 From: Stefan Zager sza...@chromium.org Date: Mon, 10 Feb 2014 16:55:12 -0800 Subject: [PATCH] Make the global packed_git variable static to sha1_file.c. This is a first step in making the codebase thread-safe. By and large, the operations which might benefit from threading are those that work with pack files (e.g., checkout, blame), so the focus of this patch is stop leaking the global list of pack files outside of sha1_file.c. The next step will be to control access to the list of pack files with a mutex. However, that alone is not enough to make pack file access thread safe. Even in a read-only operation, the window list associated with each pack file will need to be controlled. Additionally, the global counters in sha1_file.c will need to be controlled. This patch is a pure refactor with no functional changes, so it shouldn't require any additional tests. Adding the actual locks will be a functional change, and will require additional tests. Signed-off-by: Stefan Zager sza...@chromium.org --- builtin/count-objects.c | 44 ++- builtin/fsck.c | 46 +++- builtin/gc.c | 26 +++ builtin/pack-objects.c | 188 --- builtin/pack-redundant.c | 37 +++--- cache.h | 16 +++- fast-import.c| 4 +- http-backend.c | 28 --- http-push.c | 4 +- http-walker.c| 2 +- pack-revindex.c | 20 ++--- server-info.c| 35 + sha1_file.c | 35 - sha1_name.c | 18 - 14 files changed, 315 insertions(+), 188 deletions(-) I'm not really qualified to comment on substance but there are some basic style issues w.r.t. whitespace namely using 4 spaces for indent and mixing tabs/spaces. This might seem pedantic for the first round of a patch but it does put off reviewers. From Documentation/CodingGuidelines: - We use tabs to indent, and interpret tabs as taking up to 8 spaces. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
