Re: [PATCH v2 4/5] pack-objects: show some progress when counting kept objects

2018-03-16 Thread Duy Nguyen 
On Fri, Mar 16, 2018 at 8:14 PM, Duy Nguyen  wrote:
> On Mon, Mar 12, 2018 at 7:32 PM, Ævar Arnfjörð Bjarmason
>  wrote:
>>
>> On Tue, Mar 06 2018, Nguyễn Thái Ngọc Duy jotted:
>>
>>> We only show progress when there are new objects to be packed. But
>>> when --keep-pack is specified on the base pack, we will exclude most
>>> of objects. This makes 'pack-objects' stay silent for a long time
>>> while the counting phase is going.
>>>
>>> Let's show some progress whenever we visit an object instead. The
>>> number of packed objects will be shown after if it's not the same as
>>> the number of visited objects.
>>>
>>> Since the meaning of this number has changed, use another word instead
>>> of "Counting" to hint about the change.
>>
>> Can you elaborate on how the meaning has changed? With/without this on
>> linux.git I get:
>>
>> With:
>>
>> Enumerating objects: 5901144, done.
>> Getting object details: 100% (5901145/5901145), done.
>> Delta compression using up to 8 threads.
>>
>> Without:
>>
>> Counting objects: 5901145, done.
>> Delta compression using up to 8 threads.
>>
>> So now we're seemingly off-by-one but otherwise doing the same thing?
>
> Yep, it's an off-by-one bug.
>
>> As for as user feedback goes we might as well have said "Reticulating
>> splines", but I have some bias towards keeping the current "Counting
>> objects..." phrasing. We ourselves have other docs referring to it that
>> aren't changed by this patch, and there's
>> e.g. https://githubengineering.com/counting-objects/ and lots of other
>> 3rd party docs that refer to this.
>
> This is why I changed the phrase. The counting is now a bit different.
> Documents describing this exact phrase won't apply to the new version.
>
> The old way counts objects that will be packed. The new way simply
> counts objects that are visited. When you keep some packs, the number
> of objects you visit but not pack could be very high, while in normal
> case the two numbers should be the same (e.g. you pack everything you
> visit). I would prefer to print both values (e.g. "counting objects:
> /") but it's not possible with the current progress
> code.

On second thought, maybe instead of introducing a new line "getting
object details" i could just rename that line to "counting objects"?
They are exactly the same, except that in the new version, this
"counting objects" line could run a lot faster than the old line.


-- 
Duy

Re: [PATCH v2 4/5] pack-objects: show some progress when counting kept objects

2018-03-16 Thread Duy Nguyen 
On Mon, Mar 12, 2018 at 7:32 PM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Tue, Mar 06 2018, Nguyễn Thái Ngọc Duy jotted:
>
>> We only show progress when there are new objects to be packed. But
>> when --keep-pack is specified on the base pack, we will exclude most
>> of objects. This makes 'pack-objects' stay silent for a long time
>> while the counting phase is going.
>>
>> Let's show some progress whenever we visit an object instead. The
>> number of packed objects will be shown after if it's not the same as
>> the number of visited objects.
>>
>> Since the meaning of this number has changed, use another word instead
>> of "Counting" to hint about the change.
>
> Can you elaborate on how the meaning has changed? With/without this on
> linux.git I get:
>
> With:
>
> Enumerating objects: 5901144, done.
> Getting object details: 100% (5901145/5901145), done.
> Delta compression using up to 8 threads.
>
> Without:
>
> Counting objects: 5901145, done.
> Delta compression using up to 8 threads.
>
> So now we're seemingly off-by-one but otherwise doing the same thing?

Yep, it's an off-by-one bug.

> As for as user feedback goes we might as well have said "Reticulating
> splines", but I have some bias towards keeping the current "Counting
> objects..." phrasing. We ourselves have other docs referring to it that
> aren't changed by this patch, and there's
> e.g. https://githubengineering.com/counting-objects/ and lots of other
> 3rd party docs that refer to this.

This is why I changed the phrase. The counting is now a bit different.
Documents describing this exact phrase won't apply to the new version.

The old way counts objects that will be packed. The new way simply
counts objects that are visited. When you keep some packs, the number
of objects you visit but not pack could be very high, while in normal
case the two numbers should be the same (e.g. you pack everything you
visit). I would prefer to print both values (e.g. "counting objects:
/") but it's not possible with the current progress
code.
-- 
Duy

Re: [PATCH v2 4/5] pack-objects: show some progress when counting kept objects

2018-03-12 Thread Ævar Arnfjörð Bjarmason 

On Tue, Mar 06 2018, Nguyễn Thái Ngọc Duy jotted:

> We only show progress when there are new objects to be packed. But
> when --keep-pack is specified on the base pack, we will exclude most
> of objects. This makes 'pack-objects' stay silent for a long time
> while the counting phase is going.
>
> Let's show some progress whenever we visit an object instead. The
> number of packed objects will be shown after if it's not the same as
> the number of visited objects.
>
> Since the meaning of this number has changed, use another word instead
> of "Counting" to hint about the change.

Can you elaborate on how the meaning has changed? With/without this on
linux.git I get:

With:

Enumerating objects: 5901144, done.
Getting object details: 100% (5901145/5901145), done.
Delta compression using up to 8 threads.

Without:

Counting objects: 5901145, done.
Delta compression using up to 8 threads.

So now we're seemingly off-by-one but otherwise doing the same thing?

As for as user feedback goes we might as well have said "Reticulating
splines", but I have some bias towards keeping the current "Counting
objects..." phrasing. We ourselves have other docs referring to it that
aren't changed by this patch, and there's
e.g. https://githubengineering.com/counting-objects/ and lots of other
3rd party docs that refer to this.

[PATCH v2 4/5] pack-objects: show some progress when counting kept objects

2018-03-06 Thread Nguyễn Thái Ngọc Duy 
We only show progress when there are new objects to be packed. But
when --keep-pack is specified on the base pack, we will exclude most
of objects. This makes 'pack-objects' stay silent for a long time
while the counting phase is going.

Let's show some progress whenever we visit an object instead. The
number of packed objects will be shown after if it's not the same as
the number of visited objects.

Since the meaning of this number has changed, use another word instead
of "Counting" to hint about the change.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/pack-objects.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index fcdd398eb7..24af4068a9 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -44,7 +44,7 @@ static const char *pack_usage[] = {
 static struct packing_data to_pack;
 
 static struct pack_idx_entry **written_list;
-static uint32_t nr_result, nr_written;
+static uint32_t nr_result, nr_written, nr_seen;
 
 static int non_empty;
 static int reuse_delta = 1, reuse_object = 1;
@@ -1092,6 +1092,8 @@ static int add_object_entry(const struct object_id *oid, 
enum object_type type,
off_t found_offset = 0;
uint32_t index_pos;
 
+   display_progress(progress_state, nr_seen++);
+
if (have_duplicate_entry(oid, exclude, _pos))
return 0;
 
@@ -1107,8 +1109,6 @@ static int add_object_entry(const struct object_id *oid, 
enum object_type type,
create_object_entry(oid, type, pack_name_hash(name),
exclude, name && no_try_delta(name),
index_pos, found_pack, found_offset);
-
-   display_progress(progress_state, nr_result);
return 1;
 }
 
@@ -1119,6 +1119,8 @@ static int add_object_entry_from_bitmap(const struct 
object_id *oid,
 {
uint32_t index_pos;
 
+   display_progress(progress_state, nr_seen++);
+
if (have_duplicate_entry(oid, 0, _pos))
return 0;
 
@@ -1126,8 +1128,6 @@ static int add_object_entry_from_bitmap(const struct 
object_id *oid,
return 0;
 
create_object_entry(oid, type, name_hash, 0, 0, index_pos, pack, 
offset);
-
-   display_progress(progress_state, nr_result);
return 1;
 }
 
@@ -3205,7 +3205,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
}
 
if (progress)
-   progress_state = start_progress(_("Counting objects"), 0);
+   progress_state = start_progress(_("Enumerating objects"), 0);
if (!use_internal_rev_list)
read_object_list_from_stdin();
else {
-- 
2.16.2.784.gb291bd247e