search

jackalope / jackalope

Jackalope implements the PHPCR API. This modular library contains bindings to store data with the Doctrine DBAL or in the Jackrabbit JCR server part
http://jackalope.github.io
Other
265 stars 60 forks source link

Load properties of nodes lazily #303

Open danrot opened 9 years ago

danrot commented 9 years ago

In the Node class is already a comment, that the nodes should be loaded lazily. This is a real performance issue for us now, because we have 7 languages on a single node. If we want to load a single language, still all properties get hydrated.

Our solution would be to move the steps from the _setProperty method to the getProperties method, where the property gets cached. The _setProperty would then only cache the passed arguments in a separate array, which information is used to later lazily instantiate the properties in the getProperties.

If this solution would be ok for you, we would also create a PR. Just want to make sure we don't invest time in this solution, without it being merged afterwards.

dbu commented 9 years ago

i am not sure how much you can gain by that, but this was the idea, yes. if you want to work on that, great!

you would need to check that we don't have the Node class check bypass the getProperty method anywhere just in case, for example when checking its own type and the like. maybe there are a few you need to immediately load to not change the exception behaviour on inconsistencies.

for the problem at hand, child translation strategy might also be a solution, additionally saving you from even fetching all that data from the database.

danrot commented 9 years ago

If I have a look at this profile I guess the performance impact of this would be huge :smiley:

Child nodes for translation would also be a solution, but I think it's much more work than creating this PR, and this PR would also have a positive impact on other areas.

dbu commented 9 years ago

okay. cool, go for it!

danrot commented 9 years ago

Just to give you an idea of the progress (the comparisons only contain changes in jackalope):

Looks like it only pays of when loading many pages, but then there are really big improvements.

However, the code as it is now is currently not working, I cannot save anymore... That's something I'll fix the next days, then it should be ready to merge.

dbu commented 9 years ago

as the overhead should be minimal, i am +1 to have this if it helps in some cases. and these profiles look like we want this.

ping me when you updated the pull request.