Skip to content

Conversation

ernicek
Copy link

@ernicek ernicek commented Feb 17, 2020

No description provided.

@ernicek
Copy link
Author

ernicek commented Feb 17, 2020

Hello, we are using your script in our company - and our GIT contains almost 8000 files, for we want to store modified date and so on. So, because the 'applying' is the most used application, we wanted to speed up that process. And also, as we have just really quick SSD drives, make the applying multithreading was really good option.

@danny0838
Copy link
Owner

Thank you. I'm no expert about Perl threads and thus the review could take some time...

Here are some quick comments:

  1. Please remove useless trailing spaces in the code.

  2. Is there a justification about using an environment variable for thread count? Wouldn't use --mt=count or --mt with optional --mt-count be better?

  3. Do you think we really need to talk about the details of how we get CPU counts from system information in the docs? Maybe we can simply say that default thread counts is auto deduced from system information?

1) Removed info how to discover max. threads count
2) Added option to override deduced max. number of threads
@ernicek
Copy link
Author

ernicek commented Feb 18, 2020

  1. I hope i remove all of them

  2. I would say yes - And I changed to use '--mt' for enabling and '--mt-count XX' for setting the max. threads count...

  1. Changed.

@danny0838
Copy link
Owner

What about single --mt with an optional argument?

  • if option not provided, use 1 as default.
  • if option provided with no argument, use 0 for auto.
  • otherwise, use the provided count.

@ernicek
Copy link
Author

ernicek commented Feb 18, 2020

That's something that I want to usee at first... But (as I'm a newbie in PERL world) I was not able to do it... if I use this:

"mt" => 1 #default value
...
"mt:i" => \$argv{'mt'},

Then $argv{'mt'} has value 1 even the flag --mt is not specified on the command line:

$ ./git-store-meta.pl --mt
Value: 1
Died at ./git-store-meta.pl line 142.

$ ./git-store-meta.pl
Value: 1
Died at ./git-store-meta.pl line 142.

I'm pretty sure I'm doing something wrong...

@danny0838
Copy link
Owner

I get the same result that presence and absence of --mt results that default value. It seems to be introduced by Getopt::Long::Configure qw(gnu_getopt); and removing it fixes this. Though, we need to further verify removing the option won't cause a side effect.

@danny0838
Copy link
Owner

danny0838 commented Feb 21, 2020

I found a documentation for this. The issue seems to be due to the gnu_compat introduced by gnu_getopt. I think we can change Getopt::Long::Configure qw(gnu_getopt); to Getopt::Long::Configure qw(bundling permute no_getopt_compat); to allow --mt and --mt=count. Could you test and verify it?

Additionally, we just released 2.0.2 to fix a small issue. Please also rebase the work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants