django-cms / django-filer

File and Image Management Application for django
https://django-filer.readthedocs.io/
Other
1.76k stars 577 forks source link

feat: drop mptt dependency #1351

Closed fsbraun closed 1 year ago

fsbraun commented 1 year ago

Warning

This PR is for discussion. It is a proof of concept that filer can work without mptt (or any other tree library). Please do comment, but do not merge.

Description

Trees stored in databases are a pain since they can generate many database hits ("n+1" issue). django-mptt circumvents this by getting full horizontal cuts through a tree in one go, trading database hits for superfluous data that needs to be transferred and processed. To be able to do this, mptt keeps extra data for each folder (which when updating might cause race conditions).

For filer, I propose not using any tree libraries (mptt, treebeard or other), since the benefits are small

Many day to day actions such as serving files, moving folders, etc. are inherently efficient operations.

Evaluation

This discussion PR makes the case for just dropping the mptt dependency. It speaks for itself how little changes are necessary to implement it.

Please use it for tests and performance evaluation.

The test suite does not contain performance tests. Maybe we can change this?

My performance hypothesis:

Performance

Related resources

Checklist

codecov[bot] commented 1 year ago

Codecov Report

Merging #1351 (5126eca) into master (34803bc) will increase coverage by 0.06%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1351      +/-   ##
==========================================
+ Coverage   72.39%   72.46%   +0.06%     
==========================================
  Files          72       73       +1     
  Lines        3268     3258      -10     
  Branches      532      530       -2     
==========================================
- Hits         2366     2361       -5     
+ Misses        735      731       -4     
+ Partials      167      166       -1     
Impacted Files Coverage Δ
filer/admin/folderadmin.py 72.14% <100.00%> (+0.27%) :arrow_up:
...der_index_together_remove_folder_level_and_more.py 100.00% <100.00%> (ø)
filer/models/filemodels.py 86.48% <100.00%> (-0.07%) :arrow_down:
filer/models/foldermodels.py 86.86% <100.00%> (+0.39%) :arrow_up: