Sorry for delaying my report by a day. I felt it worth it, as I was finishing up on what should hopefully my final patch for the GSoC term (subject to review by other developers and subsequent editing of course) and some performance results.
The patch applies to rev. 11807. Please feel free to test it out and report any issues you experience. Just don’t trust important data to it yet.
As promised, here’s an updated patch for my MPTT implementation.
As I warned with previous patches, it’s only been tested on a relatively default install (with all sorts of artificial data of course) on my development machine. Please do not trust any vital data to it just yet. I still need to clean up some of the code I think.
In any case, feedback would be appreciated, especially on ways to prevent data corruption (as was addressed in the IRC meeting).
Early this week I spent some time cleaning up a bit of my code and documentation. It still needs some work, but I’m saving that for next week most likely. More recently I went to work on comments. We decided that overhauling the way (paged) comments are handled on the whole would be beyond the scope of my project, so they’ll get a more modest update. What that means is that I’m not entirely sure that my MPTT work will offer a significant performance boost. In fact it might actually end up slower. I’m in the process of debugging it now, so I should be testing its validity later this week.
Paged comments are proving to be much more difficult than they would seem, due to the original code’s convolution. Still trying to figure out how to adapt it to use MPTT without breaking compatibility (at least too much), and if any of the possible solutions offer enough performance gain to be worth it.
Sorry for the late update. This week wasn’t all that productive in terms of writing code. I wrote some of the code for adapting MPTT for comments (or rather the other way around), but ran into a snag for paged comments. Since they’re handled differently than categories and pages, and especially since the WP code handling them is much messier, things get a bit tricky. Most of the week has been spent in planning (and checking with Ryan to see if my plans will break stuff that they shouldn’t).
Well, a week later and I’ve got a new patch. This one includes both some fixes and updates to my previous work on categories as well as a (hopefully) fully-functioning page implementation. This one works from SVN rev. 11716. Once again, it has only undergone testing on my development system and on a near-default WordPress installation, so don’t trust any important data to it. Please to try it out on a test WordPress installation and let me know if anything is broken.
I hope to get comments finished up within the next week or so as well. From there on out it should just be testing, performance tweaking, and potentially bug-squashing.
Great work Dan!, For now, i’m reviewing how to maximize acceleration and fixing some bugs at insert nodes (by name). Can I take the code for get_lft (it is in your patch)?.
Bye bye.
Finally, some code in time for the deadline.
That patch, once applied, should prompt you for a database upgrade, automatically populate the tree with the necessary data, and use some new methods to take advantage of MPTT to display the category list (full and paged). If there are any bugs, please let me know, as I’ve only been able to test it under the conditions of my controlled environment (XAMPP for Linux).
For large numbers of categories (several hundred to a few thousand), I see a fairly consistent speedup of 8-10% with my set-up. Currently, it only works for displaying the data in the order it is stored in. Ryan and I decided that the best way to do that would be in order by name. Oddly enough, it would seem that using PHP5’s strcmp() to do the ordering produces a slightly different order than MySQL’s ORDER BY. Namely, you get the natural number ordering that Windows was so proud of a few years back (i.e. 80 comes before 100).
It still needs some polish, for instance the MPTT_Walker class still uses the old paged_walk() method, but since this seems to be used for comments only, I’m waiting until I handle those. Other things like that are scattered around. Also I still need to convert cat_rows() to be iterative rather than recursive, like I did for walk(). Other than that, the main portion of code for categories is done.
Again, feel free to test it out and let me know how it goes!
I should add, don’t use on any installations you really care about just yet. Like I said, it needs to be tested. I doubt any of you were planning on throwing it at your production site just yet, but in case you were, save yourself the trouble and just use it on a test installation for a while first.
Also of note, the patch applies to rev. 11635 (thanks for pointing it out, Rudolf). I forgot to update my SVN checkout before generating the patch.
Dan, I found a bug in my code… I watched your code and maybe your code have the same bug. Check my post in https://gsoc2009wp.wordpress.com/2009/07/08/hello-this-week-has-been-test-test/#comment-65 .
I’m not sure I follow what the bug is exactly, or rather when it occurs. The depth of the next element should simply be whatever the next element to display is, and shouldn’t depend at all on the empty terms that get_terms() left out, should it?
When hide_terms = true and hierarchical = true, get_terms delete some items of array results.
Example, if you have $cats = array(0 => A, 1=> B, 2=>C), and then you delete the item 2, the new array is $cats = array(0 => A, 2 => C).
So, for get the next depth, I use de $i + 1 item, but this doesn’t work when the array is not continuos. For example, when you are in the item $i = 0, you cannot get the next item using “$i + 1”, because the item 1 doesn’t exist.
Oh, I see now. I believe what I did was use $next->depth (from the depth stored in the database), but I’ll have to double-check.
I now have an updated patch, this time based on SVN rev. 11695. This one features the re-written mptt_cat_rows() method, as well as a bug fix. I found yesterday that my code was building the tree in the wrong order for some reason. Said reason happened to be get_terms() relying on the cached hierarchy in retrieving based on a parent ID, so I removed that section of code and it all works fine now. It might lead to a performance hit at some point though, so I’ll need to test it and see if there’s a better workaround. It’s odd that I didn’t notice it before, so I’m wondering if perhaps it was a change introduced when I updated my SVN checkout (though realistically I probably just missed it).
Just realized I left some debugging code in there that prints out some variable info when inserting new nodes in the tree. My current build isn’t diff-worthy since I’ve started working on pages already, but the pertinent code in is in the get_lft() method of MPTT_Category. You can remove the relevante lines after patching if you want to test it out and not see the debugging info. Sorry for the inconvenience.
Sorry I’m a few days late with my update. I’ve had a hectic week. Thanks to Diego’s excellent test suite (thanks, by the way :D) I’ve found some problems with my implementation that need fixing. I might be missing some sleep over the weekend to get it all fixed and ready for the midterm deadline on Monday, but it shouldn’t be anything too difficult, just time-consuming.
So it turns out that this project is more or less valid after all. I did a few simple tests displaying the front page with the default theme, using trunk and using my MPTT modifications. In each case, there was essentially just the default “Hello world!” post with a varied number of categories applied to it. Each installation had the same categories of course, and on any given test each one had the same categories selected. The MPTT installation was the clear speed winner every time. The margin depended rather significantly on both the number of categories selected and the depth at which those categories resided. I saw average page generation margins ranging from 5-40%, with the latter coming on a test using about five categories, one of which was at depth 7. It certainly wasn’t a comprehensive testing, nor was it necessarily realistic, but it shows that it is indeed worth the work.
Other than that testing, I got around to writing in some comments that I’ve been neglecting and I started writing the code for new installations and database upgrades for MPTT. Actually I should say I almost finished it. I got partway through testing when I realized that what I was doing to find top-level categories (parent == 0) would not work. It seems to me that there is no way to select top-level categories other than to select all and check each individually in a loop. This is because get_terms checks “if ($parent)” instead of “if (false !== $parent)”. I feel like it’d be a better idea to do the latter. Does anyone know if changing this will break anything or i it was designed this way for a reason? If not, I feel it would be better to change it to allow for top-level selection.
It would seem I’ve hit a milestone in my project. I believe I have written most of the new code for implementing MPTT for categories. There were surprisingly few places which actually warranted any changes for retrieval methods. I have yet to fully test the new code of course, so I’ll be working over the next few days to build up a suitable installation (lots of categories, posts that use them, etc.) and figure out which various frontend features make use of which backend functions. I’ll also need a means to gauge performance. If there already exists some plugin/script for simple page generation timing, let me know, otherwise I’ll be writing one for my own purposes. While I’m not necessarily optimizing yet, some performance testing is necessary to make sure I’m on the right track and that this approach has some validity. So that’s where I’ve been and where I’m headed to.
Hello Dan, if you have some information for make the tests, please post 😉 [I need the same think ejejeje].
Bye bye 🙂
Beau Lebens
Daryl Koopersmith
Rudolf Lai
Justin
Mark Jaquith
Matt
César D. Rodas
mike
Jake Spurlock
Summer of WordPress 
Per our discussion, double check your move_subtree() function. I haven’t tried it, but if you’ve node A and B, adjacent and both with children, ordered A, B; and switch the two so they’re ordered as B, A; or if you move B as the first/last child of A, or A as the first/last child of B;… you can potentially have edge cases that corrupt your data.