[PATCH] Allow smarter node creation based on visibility during createtree

[capnbry@…]

As I've brought up on the mailing list thread "High latency caused by full tree creation", there is a large amount of delay per LuCI request which is spent building the node tree in createtree(). Most nodes created aren't needed for the view presented to the user and only serve to consume memory and CPU time during a page load.

My idea is to provide an easy mechanism for index()ers to determine which needs to be created and what isn't. Due to the constraints of the standard LuCI web interface, this optimization needs to establish a few rules:

• The page requested must have its node created
• All parents of the page being requested must be created, so the children inherit the track
• All the top-level nodes "Status", "System", "Services", "Network" (and others added by extensions) must be created in order to have their top-level tabs in the UI
• All peers of second-level nodes need to be created as well for the same reason, to display their links on the subindexes

To make this easy to implement in each controller, the attached patch adds an "inreq" field to each created node to indicate if it lies on the request path. To satisfy the "top level node" requirement, we always add the top level node, then check its inreq property if the top-level node is not "in request", then the controller can exit index() early.

Now that we know we're in the right top level node, we can then use the same methodology per subindex. Imagine if the menus were classifications of animals

if not entry({"animal"}, template("animals"), i8n("Animals"), 10).inreq then return end

if entry({"animal", "mammals"}, template("mammals"), i8n("Mammals"), 10).inreq then
    entry({"animal", "mammals", "humans"})
    entry({"animal", "mammals", "dolphins"})
    entry({"animal", "mammals", "monkeys"})
end

if entry({"animal", "fish"}, template("fish"), i8n("Fish"), 20).inreq then
    entry({"animal", "fish", "tuna"})
    entry({"animal", "fish", "dolphins"})
    entry({"animal", "fish", "bass"})
end

This is pretty easy to use and doesn't break existing code, relying on the user to check the inreq field and omit creating nodes as necessary.

Attached patch adds this flag (to save memory it relies on nil = false so only nodes in the request path have an inreq entry in their tables).

The urltokens are removed from the request first, to prevent duplication of checking each entry of the request path for tokens which would add ridiculous overhead. If this is not acceptable to do it in httpdispatch(), I suggest that the code be moved to dispatch() before createtree() because this optimization is significant to both reduce complexity and execution time.

I've also attached a quick patch against admin-full to see it in action. The number you're looking at here is the "http waiting time" or the time between when the browser requests the page and when the web server starts sending data. I'm seeing reductions from 20-50% in waiting time (~200ms-500ms), as well as a large decrease in memory usage per request. This is just a first run of using the inreq flag; I didn't want to spend a few hours optimizing the admin pages if the patch would be completely rejected :)

Note this builds on #2476 patch's change to _create_node()

[capnbry@…]

Patch against r7342

[capnbry@…]

Patch against r7342

[jow]

I added the inreq flag infrastructure in r7357. The admin-full changes are not done yet as I plan other changes to the controllers first (get rid of redundant i18n handling).

[soma]

Maybe i'm wrong, but what will happen with drop-down menus like in some themes, won't this change break them?

[bmayland@…]

It depends on how many nodes the theme is referencing and how deep in the tree they are. If you mean they list, for example, /admin/network/wireless/radio0 while the user is in /admin/status then yes this would break that functionality as the tree is only built to /admin/network. That's not specifically the inreq flag though, it is the implementation in the network.lua controller that didn't build the child nodes.

If it can be agreed upon to which level nodes should be always built then there's a benefit if the controller code is smart enough to not collect details (such as shelling out to get wireless radio details) that wouldn't be needed if the node is being created strictly for navigation purposes instead of to actually present that view.

[capnbry@…]

There's a bug in the patch that has been integrated into svn in that nodes with a depth more than 2 do not have their inreq flag set if the parent node had not been created before the child call. The problem being in _create_node() in dispatcher.lua where the path variable is modified when passed by reference recursively.

// For request /y/z
entry({"y", "z"}) // inreq = true
// For request /a/b/c
entry({"a", "b", "c"}) // inreq = false wrong!

I've attached a patch that fixes the flag and another which updates the admin full patch to r7768.