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

[BUG] Treegrid is loading an eternity for huge amounts of data #3438

Closed
luwol03 opened this issue Jul 30, 2022 · 15 comments · Fixed by #3451
Closed

[BUG] Treegrid is loading an eternity for huge amounts of data #3438

luwol03 opened this issue Jul 30, 2022 · 15 comments · Fixed by #3451
Labels
bug Identifies a bug which needs to be addressed old user interface Issues with Old User interface
Milestone

Comments

@luwol03
Copy link
Contributor

luwol03 commented Jul 30, 2022

Problem
If you have deeply nested storage locations and viewing them via the tree view has two disadvantages.

  1. It needs a very long time to load
  2. I'ts not very readable.
    image
    (These naming makes no sense of an actual naming, but this was just the result of some demo data from me. In my real storage its not nested like that, its more like Room/rack/shelve/box/box location (one box is devided into front/middle/back). So my tree is nested very deeply and each branch contains a lot of items. And imagine a storage with 4 rooms each 12 racks each 32 boxes each splitted into 3 sections)

Suggested solution
Add a max tree depth and then lazy load the items if we try to open that specific StorageLocation.

Describe alternatives you've considered

Examples of other systems

Do you want to develop this?
If this is really wanted I would need some hints where to start.

@luwol03 luwol03 added the enhancement This is an suggested enhancement or new feature label Jul 30, 2022
@SchrodingersGat
Copy link
Member

@luwol03 how long is the API request taking to load all this data?

@luwol03
Copy link
Contributor Author

luwol03 commented Jul 31, 2022

It's taking about 10 minutes for me. I generated the data with this script:

def generate_nested(parent, loc):
    if len(loc) == 0:
        return

    cnt, name = loc.pop(0)
    print(parent.pathstring)
    for i in range(cnt):
        s = StockLocation.create(api, data={"parent": parent.pk, "name": f"{name} {i}"})
        generate_nested(s, loc.copy())


data = [
    (4, "Room"),
    (12, "Rack"),
    (32, "Box"),
    (3, "Section")
]

generate_nested(StockLocation(api, 305), data)

Timing:

Interesting, there are several requests with different timings.

  • GET /api/stock/location/tree/ - 0,294s
  • GET /api/stock/location/?parent=305&cascade=true - 23.608s

Also interesting is, that the page needs about 10 minutes to load the tree. So there must be also some client side overload.

@SchrodingersGat
Copy link
Member

10 minutes is an eternity. How many locations in that dataset?

@luwol03
Copy link
Contributor Author

luwol03 commented Jul 31, 2022

You can calculate it via 4*12*32*3=4608. I posted my generation script, if you want to try it out yourself.

@SchrodingersGat
Copy link
Member

Add a max tree depth

Do you mean that items below a certain "depth" in the tree would not be displayed in the breadcrumb tree?

@luwol03
Copy link
Contributor Author

luwol03 commented Jul 31, 2022

Do you mean that items below a certain "depth" in the tree would not be displayed in the breadcrumb tree?

Yes, and if I click on the triangle infront of the name it will lazy load the next level of the tree and display it.

@SchrodingersGat
Copy link
Member

This function is most likely the culprit for the extreme loading times:

function enableBreadcrumbTree(options) {

While the actual database query is pretty quick (~300ms, though this could probably be improved!) - this javascript function has some nasty nested loops and is in need of a major refactor.

@SchrodingersGat SchrodingersGat added this to the 0.9.0 milestone Jul 31, 2022
@SchrodingersGat SchrodingersGat added bug Identifies a bug which needs to be addressed old user interface Issues with Old User interface and removed enhancement This is an suggested enhancement or new feature labels Jul 31, 2022
@luwol03 luwol03 changed the title [FR] Breadcrumb tree lazy loading [BUG] Breadcrumb tree is loading an eternity Jul 31, 2022
@luwol03
Copy link
Contributor Author

luwol03 commented Jul 31, 2022

It looks like the the problem is not in this function. It only takes about 50ms (measured the success callback with console.time). I will try to find a solution for this problem.

But what do you think of making everything deeper than 2/3 levels collapsed and not open by default?

@SchrodingersGat
Copy link
Member

But what do you think of making everything deeper than 2/3 levels collapsed and not open by default?

I'd like to see an implementation of this, not sure exactly how it would go but am interested!

@luwol03
Copy link
Contributor Author

luwol03 commented Jul 31, 2022

@SchrodingersGat I would be happy if you could point me in the right direction about the jquery treeview function. What is that function calling? I cannot find anything useful.

@luwol03
Copy link
Contributor Author

luwol03 commented Jul 31, 2022

@SchrodingersGat I found out, that Inventree uses bootstrap-table for the tree view in combination with the tree grid plugin, right?

I did a little profiling the first 30s and It looks like this plugin is responsible for these long waiting times. Here are my results:
image
If you want to take a look yourself, here is my 1_Profile-20220731T233104.json.zip. You can import it into a chrome browser and inspect the long running functions.

I think this looks like an upstream issue.

If this is the correct repo, there are several issues about that topic, but without any reasonable response. So I think this library is more of a deprecated state and shouldn't be used anymore.

I have a few questions about Inventrees roadmap:

  1. Why does Inventree use jQuery? Isn't that more like a script which bloats the load size as many tasks can be done nowadays with plain js
  2. what is the state of switching to react as referenced here: [FR] Switch UI to React #2789 ?

@SchrodingersGat
Copy link
Member

Ah sorry when you said "breadcrumb tree" I thought you meant the navigation tree at the top of the page:

image

I see that it is the part category table which is rendering slowly. I can definitely replicate that when rendering the table in "tree view" mode.

Lazy-loading the sub items would be a good idea. We do something like this with the BOM table:

image

You can have a look at the loadBomTable method in bom.js to see how this is performed.

In particular:

function requestSubItems(bom_pk, part_pk, depth=0) {

@luwol03
Copy link
Contributor Author

luwol03 commented Aug 1, 2022

@SchrodingersGat No, I experienced the problem with the StockLocations, but I think this will exist everywhere where this type of view is available and there is a huge amount of data.

Can you please take a look at my two questions in my last post, they are somehow related to the Frontend.

@SchrodingersGat
Copy link
Member

Why does Inventree use jQuery? Isn't that more like a script which bloats the load size as many tasks can be done nowadays with plain js

I'm not sure what answer you want here :p It uses jQuery because that's what I chose when I started developing it (and, to be fair, knew absolutely nothing about front-end dev). It has served us well, and is pretty minimal in terms of overhead.

what is the state of switching to react as referenced here: #2789 ?

It is in the long term planning stage. There is a long way to go before we start such a major front-end overhaul, and it will be a huge undertaking that will probably block any other effort while it is being developed.

@luwol03 luwol03 changed the title [BUG] Breadcrumb tree is loading an eternity [BUG] Treegrid is loading an eternity for huge amounts of data Aug 1, 2022
@luwol03
Copy link
Contributor Author

luwol03 commented Aug 1, 2022

Thank you for your answers. I totally understand that switching to react will be a major change.

Sorry for my confusion, I thought the whole time, that I understand something not correctly because the function where I focused on is not responsible for what I was searching for. Yes you're right, not the breadcrumb. I mean the table treegrid.

With #3443 you reduced the request time (GET /api/stock/location/?parent=1&cascade=true) from 23s to 4s with the data generated as shown above with the script while viewing the outermost location

I tried to implement the lazy loading in #3451. Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Identifies a bug which needs to be addressed old user interface Issues with Old User interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants