? Success

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
10 Nov 2013

Optimizing JRoute

  • Allow associative arrays instead of string URIs
  • Allow routers to set the URI to HTTPS from the inside
  • some performance optimizations

Optimizing JRouter

  • deprecate JRouter::creatUri(), moving the code into the JRouter::build() method
  • optimizing code in JRouter::build()
  • introducing a runtime cache to only process a URI once per runtime
avatar Hackwar Hackwar - open - 10 Nov 2013
avatar Hackwar
Hackwar - comment - 10 Nov 2013

Here the feature tracker entry: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32653&start=0

And here the text from Joomlacode:

This proposed patch is a downsized version of my bigger JRouter-rewrite proposal. It mainly does some performance optimizations, all the time not breaking backwards compatibility in any way.

What does it do?

First of all JRoute (and subsequently JRouter::build) now accept associative arrays as input. Right now we are taking the data that we have neatly seperated into different variables, etc. and shove them into a string, only to then take that string apart again with a fairly expensive function into an array. This change allows you to directly push an array in, giving quite a performance boost when used throughout the site.

The next improvement in JRoute is, that you can define if a route should use SSL from the router and that this is not entirely controlled by the point in code where JRoute::() is invoked. For example that way, the login could always be forced to SSL from the inside of comusers router (in theory. There are other things preventing that, but its a first step)

Last improvement in JRoute is a performance improvement. Now it only uses the toString() method once instead of twice or even more often.

In JRouter, JRouter::build() gets a cache for the processed URIs. We are currently processing each URI again and again, regardless if it is the same URI, like in the title and readmore of an article. This should give quite a nice little boost to performance.

The next thing is to introduce the associative array stuff in here as well. I wanted to deprecate JRouter::createURI(), but that is not possible without doing some further rewrite and since this PR aims at 100% b/c, I wont touch this now.

In any case, this should give more flexibility and a performance boost (more or less big.)

avatar wilsonge
wilsonge - comment - 11 Nov 2013

If we're deprecating can we put deprecation notices into the doc blocks please :smile:

avatar Hackwar
Hackwar - comment - 11 Nov 2013

Actually I had to revert that deprecation, since deprecating it would mean further refactoring, which would in the end result in a potential b/c break at this point. I want to get these improvements in the core, so I removed everything that potentially poses a risk for b/c issues. Then we can later on mess around with stuff that might break the b/c after long and thorough discussions.

avatar mahagr
mahagr - comment - 11 Nov 2013

Patch breaks URLs if Apache rewrite isn't turned on (index.php goes missing on all links).

Otherwise it seems to have small, yet noticeable performance increase on pages with a lot of URLs in it. I've not yet tested with passing arrays into it, but I agree that it would allow me to skip JUri -> string -> JUri conversion, which has been a major pain to me in JRoute.

avatar Hackwar
Hackwar - comment - 12 Nov 2013

Could you check if it really is this patch that breaks that SEF behavior? Because I can't see any way how this change could create that problem...

avatar mahagr
mahagr - comment - 13 Nov 2013

I've now tested the patch with 3 different Joomla installations: "live" site, test site and clean installation. The patch breaks all of them if I turn of "URL rewriting" and remove .htaccess file.

avatar Hackwar
Hackwar - comment - 13 Nov 2013

Can you test again?

avatar mahagr
mahagr - comment - 14 Nov 2013

Now it seems to work.

avatar WebworkerBerlin
WebworkerBerlin - comment - 17 Nov 2013

Super Hannes. Wie man es von Dir gewohnt ist.
It works ^^

avatar infograf768
infograf768 - comment - 18 Nov 2013

Issues in multilang. See Tracker.

avatar betweenbrain
betweenbrain - comment - 27 Nov 2013

@Hackwar thanks for this contribution. I am excited to see improvement in this area of Joomla. What test procedures do you recommend? Is it basically to make sure nothing breaks?

avatar Hackwar
Hackwar - comment - 28 Nov 2013

Yes, for the moment it should basically not break anything. So in that regard there are no specific test instructions. Its supposed to only improve the code a bit here and there, thus not breaking anything. Most interesting would be to test it with 3rd party extensions.

avatar betweenbrain
betweenbrain - comment - 28 Nov 2013

Thanks Hannes. I would particularly be interested to see how it tests with
K2 and their advanced SET rules.

Best,

Matt Thomas
Founder betweenbrain™
Lead Developer Construct Template Development Framework
Phone: 203.632.9322
Twitter: @betweenbrain
Github: https://github.com/betweenbrain

Sent from mobile. Please excuse any typos and brevity.
On Nov 28, 2013 12:30 PM, "Hannes Papenberg" notifications@github.com
wrote:

Yes, for the moment it should basically not break anything. So in that
regard there are no specific test instructions. Its supposed to only
improve the code a bit here and there, thus not breaking anything. Most
interesting would be to test it with 3rd party extensions.

—
Reply to this email directly or view it on GitHub#2487 (comment)
.

avatar betweenbrain
betweenbrain - comment - 2 Dec 2013

@test tested with K2, with it's advanced SEF enabled, as well as with sh404sef. No apparent issues with them using a single language.

avatar infograf768
infograf768 - comment - 13 Dec 2013

Still testing on multilang. No issues for now. Anyway to measure the improvements?

avatar mahagr
mahagr - comment - 14 Dec 2013

The best way to measure the improvement is to have a page with 1000 or so internal SEF links. The difference is small but measurable.

avatar Hackwar
Hackwar - comment - 14 Dec 2013

@infograf768 As @mahagr said, that would be the best way to do this. In general, I'm not aiming for a huge performance gain here, but small incremental improvements. Considering our latest discussions, I will look into better documentation and unittests before this should be merged.

One of the advantages would be, that you can set a URL to HTTPS from inside the component router for example. Right now, you have to define if it should be HTTPS when calling JRoute::() in your code. That said, you might wanna use HTTP for your shoppingcart or you want to use HTTPS. Instead of implementing that switch as a config option in every JRoute::(), you would put this into your router at one point. The router sees your URL and says "This needs to be SSL!" and toggles the switch. Besides making coding easier, it also means that you can't forget one link in your component or content that maybe does not have the currently necessary config switch implemented. :-)

avatar Hackwar
Hackwar - comment - 13 Feb 2014

oh, f#!k this. Changing the target for a PR seems to be impossible. I'm going to close this one and create a new PR later this afternoon.

avatar Bakual
Bakual - comment - 13 Feb 2014

You can close this, and open a new PR from same branch then. That works.

avatar Hackwar
Hackwar - comment - 22 Feb 2014

created a new PR with #3164

avatar Hackwar Hackwar - change - 22 Feb 2014
Status New Closed
Closed_Date 0000-00-00 00:00:00 2014-02-22 23:10:02
avatar Hackwar Hackwar - close - 22 Feb 2014
avatar Hackwar Hackwar - close - 22 Feb 2014
avatar Hackwar Hackwar - head_ref_deleted - 6 Jan 2016

Add a Comment

Login with GitHub to post a comment