User tests: Successful: Unsuccessful:
Optimizing JRoute
Optimizing JRouter
If we're deprecating can we put deprecation notices into the doc blocks please
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.
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.
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...
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.
Can you test again?
Now it seems to work.
Super Hannes. Wie man es von Dir gewohnt ist.
It works ^^
Issues in multilang. See Tracker.
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.
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)
.
Still testing on multilang. No issues for now. Anyway to measure the improvements?
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.
@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. :-)
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.
You can close this, and open a new PR from same branch then. That works.
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-02-22 23:10:02 |
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.)