Jump to content

nik

PW-Moderators
  • Posts

    294
  • Joined

  • Last visited

  • Days Won

    4

Everything posted by nik

  1. PHP-newbie just had to find this one out - but it wasn't actually that much of a PHP-related thing after all... Yes, the leading zero makes it octal - and octal numbers may contain only digits 0-7, so your example was just a bit off the limits . PHP.net says: "If an invalid digit is given in an octal integer (i.e. 8 or 9), the rest of the number is ignored.". (Whole article on integers.) And there's the explanation for 008 becoming a zero in the quote as well.
  2. This one's just great, Soma! Had to give it a try straight away. Same goes for site/assets/ as well. Without write permissions downloading the module zip file fails. This actually revealed a bug too, giving this: Error Using $this when not in object context (line 278 of /Users/nik/Sites/test/site/modules/ModulesManager/ModulesManager.module) Looks like you're using $this inside a static method (downloadFile()). Changing $this->error(...) to wire()->error(...) does the trick (don't know if this is the best way, though). There seems to be a few of these actually in the same method. This left me a bit confused at first as there was no new page under "Setup" after installation. But then I found it under "Modules". So, either that's what you intended and just documented it wrong or maybe page with id 21 isn't always "Setup". Either way, something should be fixed here as well . Anyway, good work so far!
  3. Just wanted to say that the memory consumption issue I mentioned in my previous post most probably was about having debug mode on (as I usually do on a development site). So that part seems to be ok as well. And thanks go to Ryan once again - just read about this from a tweet by @processwire .
  4. Ryan, Great, it seems to me you nailed it! My own tests now execute as they should, not slowing down anymore as there are more pages. And the new code in Pages.php makes more sense to me as well. So at least no obvious flaws there I can find. Creating pages with repeaters does take more time than creating pages without them, but as you said that's expected behaviour. This has a huge impact on the project I've been working on. Actually, I had to drop repeaters from first version because of this performance issue. I think I didn't mention it in the beginning, but on top of being unbearably slow, memory consumption was not at an acceptable level either when handling large amount of pages. I haven't tested this part yet, but I'll sure be telling you if this didn't get fixed as well (wouldn't be that surprising if it actually did). Anyway, now I'm able to return to my original plan and utilize repeaters the way they deserve to be utilized. I also ran your test package successfully. One comment on the script though. The very first get()-call trusts the data has been populated in right order: If "do.php repeater-populate" is run before "do.php populate", this will return country-97 under /countries-repeater/ and fail at the second move (Chosen parent '/countries/' already has a page named 'country-97'). So has_parent=/countries/ or something else needs to be added there - or of course one can populate everything in the right order.. I have to say I really like the work you're doing with ProcessWire - keep it up. Once I managed to get the issue out in the public, it got instantly taken care of. So, thanks! More posts to come from here now that I've started . I'm glad to be of help wherever I can.
  5. Yes, it's just a bottleneck. Everything works right and data is correct in the database. The original database has customer data in it, but I created a test environment where I'm able to reproduce all of this. Working from a fresh installation of PW 2.2.2 this is how to get there (for those who are interested): Setup #1 create page /countries/ measure time taken to save a new child page (./do.php test) add 200 countries with 2 cities in each of them ./do.php populate) measure time taken to save a new child page again (./do.php test) --> page creation slowed down with a factor of 5-15 (varies of course depending on the environment) Setup #2 install module "FieldtypeRepeater" create a repeater field "dummy", add field "title" to it (not really a useful repeater...) create a no-file template "basic-page-with-repeater" (duplicate fields from "basic-page") add field "dummy" to template "basic-page-with-repeater" create page /countries-repeater/ measure time taken to save a new child page (./do.php repeater-test) add 100 countries with 50 cities in each of them (./do.php repeater-populate) measure time taken to save a new child page again (./do.php repeater-test) --> page creation slowed down with a factor of 100 or more And here's the dirty little script (do.php) I'm referring to above. Place it in "site" directory (and fix the shebang if needed). #!/usr/bin/php <?php include("../index.php"); switch($argv[1]) { case 'populate': populate(); break; case 'test': test(); break; case 'repeater-populate': repeaterPopulate(); break; case 'repeater-test': repeaterTest(); break; default: echo "Unknown op {$argv[1]}\n"; break; } function addPage($parent, $name, $tpl='basic-page') { $p = new Page(); $p->parent = $parent; $p->template = $tpl; $p->title = $name; $start = microtime(true); echo "addPage $name\n"; $p->save(); $diff = microtime(true) - $start; printf(".. {$p->path} saved in %.6f seconds\n", $diff); return $p; } function populate() { $container = wire('pages')->get("/countries/"); foreach(range(1, 100) as $countryN) { $country = addPage($container, "country-$countryN"); foreach(range(1, 2) as $cityN) { $city = addPage($country, "city-$cityN"); } } } function test() { addPage(wire('pages')->get('/countries/'), 'test'); } function repeaterPopulate() { $container = wire('pages')->get("/countries-repeater/"); foreach(range(1, 100) as $countryN) { $country = addPage($container, "country-$countryN", "basic-page-with-repeater"); foreach(range(1, 50) as $cityN) { $city = addPage($country, "city-$cityN", "basic-page-with-repeater"); $rep = $city->dummy->getNew(); $rep->title = 'repeater title'; $city->save(); } } } function repeaterTest() { $test = addPage(wire('pages')->get('/countries-repeater/'), 'test', 'basic-page-with-repeater'); $rep = $test->dummy->getNew(); $rep->title = 'repeater title'; $test->save(); } Ryan, I also emailed you the database dump with fields, templates, and container pages set up, so that you could work from exactly where I'm at (starting from the actual tests in both setups). No, I haven't had time to go there yet. But I will do that unless you came up with a working solution before . Well, yes. That's where I started from =). Here's a git diff for Pages.php with debug timers and output. Put it in place before populating and you'll notice the strange behaviour immediately. diff --git a/wire/core/Pages.php b/wire/core/Pages.php index c40042d..1b96c7c 100644 --- a/wire/core/Pages.php +++ b/wire/core/Pages.php @@ -565,7 +565,11 @@ class Pages extends Wire { } // if entries aren't present, if the parent has changed, or if it's been forced in the API, proceed if($n == 0 || $page->parentPrevious || $page->forceSaveParents === true) { + $start = microtime(true); + echo " saveParents({$page->id})\n"; $this->saveParents($page->id, $page->numChildren + ($isNew ? 1 : 0)); + $diff = microtime(true) - $start; + printf(" ..took %.6f seconds\n", $diff); } } @@ -660,7 +664,11 @@ class Pages extends Wire { ); while($row = $result->fetch_array()) { + $start = microtime(true); + echo " recursion: saveParents({$row['id']})\n"; $this->saveParents($row['id'], $row['numChildren']); + $diff = microtime(true) - $start; + printf(" ..took %.6f seconds\n", $diff); } $result->free(); As you can see, saveParents() gets called extensively. There rows get first deleted and the inserted again to table pages_parents. I checked the rows in that table before and after inserting a new page with setup #2 and the only difference was parents information for the newly created page - as expected. Niklas
  6. I've been diving into the core of PW trying to understand performance problems when there are lots of pages, especially ones with repeater fields in their templates. The problem is that creating a page takes seconds (rather than milliseconds) when certain conditions are met. Setup #1 one container page, say /countries/ hundreds of subpages with children, /countries/<country>[/<state>]/<city>/<data> for example create a new page, parent being /countries/ Setup #2 a large page hierarchy with not that many pages directly under one parent, but hundreds of pages with template(s) that each have the same repeater field structure becomes flat again as repeaters use internally page hierarchy of this kind: /processwire/repeaters/for-field-123/for-page-456/1234567890-1234-1 (--> .../for-page-nnn/... are all siblings even when the actual pages are not) create a page with a repeater field used all over the site, parent is irrelevant And of course I've got both of these on one site . Now, what I don't quite understand is what the Pages class method ___save() does just before calling some hooks in the end (from: wire/core/Pages.php): if(($isNew && $page->parent->id) || ($page->parentPrevious && !$page->parent->numChildren)) { $page = $page->parent; // new page or one that's moved, lets focus on it's parent $isNew = true; // use isNew even if page was moved, because it's the first page in it's new parent } if($page->numChildren || $isNew) { // check if entries aren't already present perhaps due to outside manipulation or an older version $n = 0; if(!$isNew) { $result = $this->db->query("SELECT COUNT(*) FROM pages_parents WHERE parents_id={$page->id}"); list($n) = $result->fetch_array(); $result->free(); } // if entries aren't present, if the parent has changed, or if it's been forced in the API, proceed if($n == 0 || $page->parentPrevious || $page->forceSaveParents === true) { $this->saveParents($page->id, $page->numChildren + ($isNew ? 1 : 0)); } } So, for new (and moved) pages it always ends up calling saveParents() for the parent page of the page being saved. saveParents() then works recursively deeper into the tree calling itself for every child page with children of its own. Given the setup #1 this means going through every .../<country>/ and .../<state> and .../<city> page there is. On the other hand, with setup #2 every single repeater instance (.../<for-page-nnn>/) with data inside will be handled. It doesn't really surprise me anymore this takes a while: saveParents() is deleting rows and inserting new ones into pages_parents for thousands of pages, unnecessarily if I'm right. It does seem necessary to call saveParents() for the parent page if the page being saved is its first child (pages_parents has a row for each ancestor of the parent page, but not for the page or parent page itself), but the recursion goes all over the place. I think it should be somehow restricted to the new/moved page and its children (and so on) only. Also, the second argument to saveParents() gets a wrong value when $page refers to the parent page as numChildren has already been incremented for the parent page (though this doesn't actually break anything). As long as new pages can't have children when they're saved for the first time, saveParents() shouldn't be called at all when saving a new page. If child pages may exist, it isn't of course that straightforward but still the saveParents() recursion should be restricted as stated above. I hope my explanation makes sense to you. It's quite possible there is something laying under the hood that I just can't see now having studied this thing for a while (well, hours). But it sure seems like a bug to me .
×
×
  • Create New...