Skip to content

Conversation

@LmanTW
Copy link
Contributor

@LmanTW LmanTW commented Jul 6, 2025

Hello to the OpenList community! This is an attempt to implement directory size for the local driver.

The size of directories are pre-calculated at initialization, and updated when user modify files through OpenList. Because of this if files are modified not using OpenList, the size displayed will likely be incorrect (can be fix by reloading the storage).

I don't know how well this would scale, so by default this option is set to false.

@jyxjjj
Copy link
Member

jyxjjj commented Jul 9, 2025

Thank you for your contribution and the detailed explanation!

  • Pre-calculating all directory sizes at initialization can significantly increase load time, especially for large or deeply nested folder structures, which negatively impacts user experience.

  • And as you said, the accuracy of folder sizes depends on users performing all file operations through OpenList, which is hard to guarantee in real-world scenarios.

Because of these concerns, we believe the current approach needs further refinement to balance performance and reliability. We’d suggest:

  • Exploring lazy or on-demand size calculation;

  • Providing a manual refresh option instead of automatic updates;

Or packaging this as an optional setting for users who need it.

We really appreciate your effort and welcome further improvements on this feature!

@LmanTW
Copy link
Contributor Author

LmanTW commented Jul 9, 2025

Hello, thanks for the response!

  • First, I'm not sure how "Exploring lazy or on-demand size calculation" would be possible. I'll assume this mean doing a full folder size calculation lazily. But to do a full folder size calculation, the program would still need to scan through every single children under the folder recursively no matter what, and I don't think there's any other way to do it.

  • Second, the administrator can already do manual refresh through reloading the storage (yes, currently not clear that you can do that). I don't think the user (people using the hosted OpenList) should be able to do this, because people can abuse this into overwhelming the server.

  • Third, the implementation is in fact, already optional. The administrator can enable/disable it via the storage settings, and it default to not calculating the size of directories.

I'm at the concussion that this implementation is already as performant as it can be, it already minimized the calculation which need to be done after the first initialization. As of reliability, I think it can be improved by listening to FS events, but I'm not sure if that work well with OpenList's way of implementing drivers.

@PIKACHUIM
Copy link
Member

Hello, thank you for your contribution
Due to the involvement of complex and recursive computations,
We need a performance report to assess its impact on performance

@LmanTW
Copy link
Contributor Author

LmanTW commented Jul 24, 2025

Thanks for the response, do we have any official performance testing methods? Or I just need to test it myself on large amount of files.

@PIKACHUIM
Copy link
Member

Not yet. We will test the impact on performance ourselves.
If the evaluation shows no issues with this pr, we will merge it.
Thank you for your patience.

@jyxjjj jyxjjj requested a review from Copilot August 6, 2025 14:10

This comment was marked as outdated.

Copy link
Member

@dezhishen dezhishen left a comment

Choose a reason for hiding this comment

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

尝试使用ide的格式化代码功能,对代码进行格式化。
或者使用go fmt进行格式化

@jyxjjj jyxjjj requested a review from Copilot August 7, 2025 07:08

This comment was marked as outdated.

@ILoveScratch2 ILoveScratch2 requested a review from Copilot August 7, 2025 08:40

This comment was marked as outdated.

Signed-off-by: 我怎么就不是一只猫呢? <26274059+dezhishen@users.noreply.github.com>
…endless loop

Signed-off-by: 我怎么就不是一只猫呢? <26274059+dezhishen@users.noreply.github.com>
Signed-off-by: 我怎么就不是一只猫呢? <26274059+dezhishen@users.noreply.github.com>
@dezhishen dezhishen requested a review from Copilot August 8, 2025 02:47

This comment was marked as outdated.

@ILoveScratch2 ILoveScratch2 requested a review from Copilot August 8, 2025 03:10

This comment was marked as outdated.

dezhishen
dezhishen previously approved these changes Aug 8, 2025
@jyxjjj
Copy link
Member

jyxjjj commented Aug 8, 2025

All review conversations must be marked as resolved before merging.
Btw, how are the performance tests going?

@jyxjjj jyxjjj requested a review from ILoveScratch2 August 8, 2025 03:28
ILoveScratch2
ILoveScratch2 previously approved these changes Aug 8, 2025
Copy link
Member

@ILoveScratch2 ILoveScratch2 left a comment

Choose a reason for hiding this comment

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

性能可以就行

@dezhishen dezhishen dismissed stale reviews from ILoveScratch2 and themself via 84c3250 August 8, 2025 06:07
@dezhishen
Copy link
Member

go test -benchmem -run=^$ -bench ^BenchmarkCalculateDirSize$ github.com/OpenListTeam/OpenList/v4/drivers/local -cpu 1 -benchtime=100x
goos: windows
goarch: amd64
pkg: github.com/OpenListTeam/OpenList/v4/drivers/local
cpu: Intel(R) Core(TM) i5-10400F CPU @ 2.90GHz
BenchmarkCalculateDirSize            100          10918770 ns/op           97759 B/op        488 allocs/op
--- BENCH: BenchmarkCalculateDirSize
    benchmark_calculatedirsize_test.go:50: Starting performance test for directory size calculation
    benchmark_calculatedirsize_test.go:90: Directory size: 6656000 bytes
    benchmark_calculatedirsize_test.go:91: Performance test completed successfully
    benchmark_calculatedirsize_test.go:50: Starting performance test for directory size calculation
    benchmark_calculatedirsize_test.go:90: Directory size: 6656000 bytes
    benchmark_calculatedirsize_test.go:91: Performance test completed successfully
PASS
ok      github.com/OpenListTeam/OpenList/v4/drivers/local       3.087s

@dezhishen dezhishen requested a review from jyxjjj August 8, 2025 06:13
jyxjjj
jyxjjj previously approved these changes Aug 8, 2025
@dezhishen dezhishen merged commit 93c0621 into OpenListTeam:main Aug 8, 2025
12 checks passed
@LmanTW
Copy link
Contributor Author

LmanTW commented Aug 8, 2025

感謝各位大佬的協助!

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.

6 participants