Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added feedback after each roughly 100,000 file system objects. #3

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

HumanJHawkins
Copy link

Added a separate count for tracking when to give feedback. Used this method instead of modulus operator in attempt to stay true to the original intent of this being an extremely fast implementation. Yet needed some indication that the program is not stalled during count of entire drives, etc.

I notice my IDE removed some line ending whitespace, making the default diffing at github difficult. To ignore whitespace changes during review at github, add ?w=1 to the URL. For example:
https://github.com/ChristopherSchultz/fast-file-count/compare/master...HumanJHawkins:master?w=1

Thanks for considering this change.
Jeff

Added a separate count for tracking when to give feedback. Used this method instead of modulus operator in attempt to stay true to the original intent of this being an extremely fast implementation. Yet needed some indication that the program is not stalled during count of entire drives, etc.
@ChristopherSchultz
Copy link
Owner

I'm trying to decide if I want to merge this. In general, I like the idea. On the other hand, it's quite arbitrary what that feedback limit is... you want 100,000, others might want 1M, etc. Adding command-line options complicates the program, while recompiling is not terribly convenient.

Given that this is essentially a demonstration program for programmers, I think it would be okay to use a compile-time constant. Can you adjust your PR to #define a constant for the magic-number when feedback is given?

I'm about to commit a few changes to the master, so please make sure you pick those up.

@HumanJHawkins
Copy link
Author

Understood. I had a similar concern. More of an aversion to adding anything to what is intended to be an extremely simple and fast demo. But decided to PR this more as an indication that it hasn't hung than as a feedback on quantity.

Will grab any changes and look at adding the #define with minimal additional complexity. A good implementation for the end-user would involve being able to turn it off, and mitigating or warning about potential choices that would slow it down. Like getting feedback after every file. But that would be a poor implementation for the demo-oriented purpose of this program.

So perhaps just a #define, with comments in the code to indicate how to effectively turn it off (setting a really large number) and the problems caused by a really small number (i.e. slowness)?

Hope to revise the PR soon... May not look at it until tonight.

@ChristopherSchultz
Copy link
Owner

Sounds good. The "slowdown" would only be detectable if the user set the value to something really low, and lots of stdout messages were being dropped. Reading the filesystem is going to be the bottleneck in this program, not incrementing a long value.

@HumanJHawkins
Copy link
Author

I believe I've implemented this as suggested. As compared to the earlier code, the following changes have been made:

  1. FEEDBACK_INTERVAL is set via #define
  2. Simplified the output... Feedback and final result now flow smoothly from one to the other via refreshing/updating a single line.
  3. Made default interval 10,000, as this seems a better fit for the simplified output.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants