Skip to content

Conversation

@uvery9
Copy link

@uvery9 uvery9 commented Aug 8, 2021

when >= FILES_MAX, do not use this Plugin, will cost too much time.

@adyanth
Copy link
Owner

adyanth commented Aug 8, 2021

Hey @JareddC thanks for the PR!

Had a question here. How did you choose the value for FILES_MAX? I created a folder with 500 files and it barely took a second to show all of them. I'm just thinking that with git repos and all having lots of tiny files, 300 might get exceeded rather too quickly. Running the plugin on my Program Files, it took around 2 seconds for 100,000+ files.
image

IEnumerable<string> subDirs = Directory.EnumerateDirectories(rootPath);
foreach (string dir in subDirs)
{
if (foundFiles.Count() < FILES_MAX)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you break (or even return) if the condition fails, so that each recursive call can travel up as soon as it exceeds FILE_MAX?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Less than FILES_MAX, just return the file count of folders. Use try, catch to catch UnauthorizedException and so on.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I did not get it.

What I am trying to say is, if the condition fails, and the count exceeds, add an else block to either break or return so that we stop additional looping.

@uvery9
Copy link
Author

uvery9 commented Aug 9, 2021

Hey @JareddC thanks for the PR!

Had a question here. How did you choose the value for FILES_MAX? I created a folder with 500 files and it barely took a second to show all of them. I'm just thinking that with git repos and all having lots of tiny files, 300 might get exceeded rather too quickly. Running the plugin on my Program Files, it took around 2 seconds for 100,000+ files.
image

This is my point of view.
When there are too many files, it is not appropriate to display in the form of a directory, or use a depth form like 'tree -L 3' command in Linux to show less files. tree command
300 may be a more appropriate value.

@adyanth
Copy link
Owner

adyanth commented Aug 9, 2021

Oh, do you mean more than 300 files in the same directory?

If that is the case, correct, it is not really appropriate. But, it is not appropriate for that subfolder if it was inside. Do we not want to show the parent folder too in that case?

I will try once with your patch when I have time to see the behaviour and will let you know.

@uvery9
Copy link
Author

uvery9 commented Aug 9, 2021

fix pack-zip.ps1 error by
cd "$(SolutionDir)Scripts"
powershell -file "$(SolutionDir)Scripts\pack-zip.ps1"

@adyanth
Copy link
Owner

adyanth commented Aug 14, 2021

Here is an example, there are no folders with more than 300 files here. I would really like to be able to open the folder to go to the bin folder and open the executable directly. Seems too limiting. If it was a limit of 300 files only in that folder, I would see that making sense. But then, it would need to have lazy loading as and when sub folders are expanded to have any advantage.

image

If you would like to use it, here is the link to the build. Let me know if it suits you. At this point, I do not feel it is a useful limitation to impose. Let me know if you have other thoughts on this.

@uvery9
Copy link
Author

uvery9 commented Aug 16, 2021

When there are dozens(may be just 30) of video files in the folder, this plugin will be really slow

@adyanth
Copy link
Owner

adyanth commented Aug 16, 2021

Okay, just verified that now, it is true. Looks like the thumbnail generation takes time for video files. @mooflu thoughts on this issue?

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