Decrease load time

[nWidart]

The sidebar builder takes approx. 300ms to load the sidebar. I have 12 menu items.

That is pretty much the same as the Laravel framework.

Maybe run it into a profiler or something, to find where there is a hog.

[michaeljhopkins]

I noticed the service provider is quite a bit different than what I have seen in the past. Would registering/booting this package similar to how dwightwatson/bootstrap-form does help in resolving this?

*Service providers are absolutely not my strength, so this is largely a shot in the dark

[patrickbrouwers]

No, the ServiceProvider is optimal like it is right now. Deferred loading won't help, because the manager should register the groups and items on every load, which makes defer not useful. The SidebarManager needs to be a singleton, because every time it get's called it should be the same instance.

Did some testing, with 100 menu items divided through 6 groups with all of them given an append and a badge and with rendering included, the script finishes in 89.31ms.

[nWidart]

Yup, I think this can be closed. :)

Thanks for the profiling. :smiley:

[nWidart]

Going to re-open this.

After doing some profiling on blackfire.io : https://blackfire.io/profiles/fdadcba6-f05c-4549-8108-d52d73b421b0/graph

93.9 ms is spent on Maatwebsite\Sidebar\SidebarManager group 78.8 ms is spent on Maatwebsite\Sidebar\SidebarManager addItem

[patrickbrouwers]

Any suggestions on how to speed it up?

[mvdstam]

I don't think it's necessarily Sidebar being slow here. Both SidebarManager::group and Itemable::addItem take a Closure argument, thus executing stuff that may be slow on itself. That said, Itemable::addItem also depends on Laravel's RouteDependencyResolverTrait and does dependency resolving, which may be costly as well.

Perhaps the rendered result could be cached? I'm not sure why you would want to do the whole rendering thing on every request anyway, especially in cases where sidebars are pretty much static.

[FrankPeters]

Yeah caching would be a good choice anyway. That's up to the application (as user rights could be a factor etc.).

[patrickbrouwers]

User rights and active state would be a problem when caching.

The way you check the user rights can be the thing that makes things slow.

The closures should be optional. Possibly the depenecy resolver can become optional. However I think it doesn't do a lot when not used anyway.

The user could potentially implement the caching himself, but be aware of activate states and user rights, something which we use a lot in our applications.

[mvdstam]

I think the presentation layer should be more seperated (and if possible, completely seperated) from the business logic layer (in this case: building and generating the whole structure of the sidebar). For example, the SidebarManager::render() method could be seperated completely into a different layer of code, which would simply take a collection of structured data as determined by the SidebarManager itself (called the $this->groups property from what I can see). That data could be cached pretty nicely that way without affecting view-logic.

Just thinking out loud here. But I think the SidebarManager kind of developed into a god-object which makes the whole thing hard to scale when it comes to performance. The whole closures and dependency resolving thing is pretty neat and shouldn't be sacrificed. Instead, the focus should be more on the architecture of some of the components. Lastly, Laravel has a terrific system for both caching and events, which should make flushing the cache when necessary not a hard thing to accomplish. But stuff like active states is something that belongs in the view in my opinion; not in other layers of your application.

Thoughts?

[patrickbrouwers]

Interesting point. Will think about it. I agree that it would be best to separate the business logic.

Possibly a good idea to move the activate state check to the view logic. Would make it easier to cache.

Will return with some more ideas.

[nWidart]

New profile for asgardcms.com running version 2.0: https://blackfire.io/slots/55ca0cd1-b01f-4556-97de-34bbbe8e1f15/graph

Laravel-Sidebar is still the slowest point of the app.

[nWidart]

Here is everything related to the package:

in order:

• 34.9ms
• 24.9ms
• 15.1ms
• 9.63ms
• 6.49ms
• 6.31ms
• 3.47ms
• 1.75ms

That's a total of 102,54ms. :astonished:

[patrickbrouwers]

A lot faster then it was then. And with caching enabed it will only be 10ms (In my tests)

[nWidart]

Did you test with blackfire ? :smile: Since caching isn't ready to use, it can't be used yet though.