Smalyshev added a comment.
I believe 1.5% slowdown is acceptable for the functionality. I will review the code soon and add my comments.TASK DETAILhttps://phabricator.wikimedia.org/T162371EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: SmalyshevCc: hoo,
WMDE-leszek added a comment.
Thanks @Ladsgroup!
I've copied over the table to the task description, so it is better visible. I've also added links to the actual patches tested, and added a column with some ratio measuring somehow how slower was the patched code compared to master.
@daniel,
Ladsgroup added a comment.
I did a table of running it on master, the old patch and the alternative one.
NameFirst runSecond runThird runAverage
Master19.9021.09121.09620.696
Old patch23.30123.15622.93123.129
Alternative patch20.96520.71621.33721.006
TASK
Ladsgroup added a comment.
With the alternative approach:
Without it (on master):
root@federated-wikis:/var/www/html# time php extensions/Wikibase/repo/maintenance/dumpRdf.php > /dev/null
Dumping shard 0/1
...
Processed 7042 entities.
real 0m21.666s
user 0m18.016s
sys 0m0.376s
WMDE-leszek added a comment.
Thanks @daniel and @Smalyshev for the feedback so far.
Submitted https://github.com/wmde/WikibaseDataModel/pull/767 to get those changes made. Feel free to review and comment wherever you find applicable.TASK DETAILhttps://phabricator.wikimedia.org/T162371EMAIL
Smalyshev added a comment.
The benchmark looks pretty good. I'll review the patches a bit later (I have a cold right now and it's hard for me to concentrate) and if the code is OK I see no reason not to merge it. We could also do another xhprof run before that to see if there are any other things
daniel added a comment.
@WMDE-leszek The approach looks fine, and should help. The question is if it's sufficient.TASK DETAILhttps://phabricator.wikimedia.org/T162371EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: danielCc: hoo, Ladsgroup, PokestarFan,
WMDE-leszek added a comment.
Thanks @Ladsgroup for running another round of benchmark.
To make it clear what has been actually benchmarked. I've update the same patch that has been subject to analysis before, i.e. https://gerrit.wikimedia.org/r/#/c/345332/22. The patch itself has not really been
Ladsgroup added a comment.
I did another round of benchmarking with @WMDE-leszek's new patch and the result is like this:
Without it:
root@federated-wikis:/var/www/html# time php extensions/Wikibase/repo/maintenance/dumpRdf.php > /dev/null
Dumping shard 0/1
...
Processed 7042 entities.
real
Smalyshev added a comment.
I've introduced a cache to reduce number of calls to EntityId::getLocalPart and EntityId::getRepositoryName.
I think we also need to take a good look why splitSerialization is getting called so much. We do not need to do any transformations to the IDs we get from the
WMDE-leszek added a comment.
Thanks @Smalyshev for looking into this! I've just submitted a new patchset of https://gerrit.wikimedia.org/r/#/c/345332 where I've introduced a cache to reduce number of calls to EntityId::getLocalPart and EntityId::getRepositoryName.
As I am really not an expert
Smalyshev added a comment.
Some analysis:
Master run:
https://items-repo.wmflabs.org/xhprof_html/?run=598b8f9c66fab=dumpRdf=excl_wt
Patched run:
https://items-repo.wmflabs.org/xhprof_html/?run=598b8d44818ce=dumpRdf=excl_wt
We can see that one of the most expensive functions there is
Ladsgroup added a comment.
It's amazing. Thank you!TASK DETAILhttps://phabricator.wikimedia.org/T162371EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: LadsgroupCc: hoo, Ladsgroup, PokestarFan, Lucas_Werkmeister_WMDE, Smalyshev, daniel, WMDE-leszek, Aklapper,
Smalyshev added a comment.
GUI seems to work now, see https://items-repo.wmflabs.org/xhprof_html/index.php?run=5984fc915da57=dumpRdf
new runs should be added to https://items-repo.wmflabs.org/xhprof_html/ as they happen. On the filesystem they reside on /var/tmp/xhprof, feel free to clean up those
Ladsgroup added a comment.
@Smalyshev: I installed XhGui (also with mongodb) but still I get this strange error PHP Fatal error: Class 'XHProfRuns_Default' not found in /var/www/html/extensions/Wikibase/repo/maintenance/dumpRdf.php on line 139. I couldn't find any place to fix it.TASK
daniel added a comment.
In T162371#3487544, @Ladsgroup wrote:
@daniel: The paste file is 35K lines and it's bringing my laptop to its knees :( Can you edit your comment and make it link?
done. sorry!TASK DETAILhttps://phabricator.wikimedia.org/T162371EMAIL
Ladsgroup added a comment.
@daniel: The paste file is 35K lines and it's bringing my laptop to its knees :( Can you edit your comment and make it link?
@Smalyshev: Yeah, in ssh federated-wikis.eqiad.wmflabs file /var/www/html/extensions/Wikibase/repo/maintenance/dumpRdf.php I added the lines for
Smalyshev added a comment.
xhprof without GUI is kinda hard to comprehend... where it is installed? I could try to make the GUI work.TASK DETAILhttps://phabricator.wikimedia.org/T162371EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: Ladsgroup, SmalyshevCc:
daniel added a comment.
To be honest, I have no idea how to make sense of the raw data, and I can't find a visualizytion tool that would take it as input.
@Smalyshev, do you have an idea?TASK DETAILhttps://phabricator.wikimedia.org/T162371EMAIL
Ladsgroup added a comment.
I sent you the raw report, Couldn't make the GUI to work.TASK DETAILhttps://phabricator.wikimedia.org/T162371EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: LadsgroupCc: hoo, Ladsgroup, PokestarFan, Lucas_Werkmeister_WMDE,
daniel added a comment.
@Ladsgroup thank you! So, this looks like a 10-15% increase. Not too horrible, but significant.
It would be good to learn what exactly is causing it. Can you run this though xhprof and see what the bottle necks as in the methods that were changed by the patch?TASK
Ladsgroup added a comment.
Things to remember:
In props-repo, properties are local and items are coming from the other repo and vice versa.
Database of both of them lives in item-repos instance for simplicity reason, one of the reasons that it takes some time is that it tries to read a remote
Ladsgroup added a comment.
First run on props-repo with the patch:
ladsgroup@federated-wikis2:/var/www/html$ time php extensions/Wikibase/repo/maintenance/dumpRdf.php > /dev/null
Dumping shard 0/1
...
Processed 7034 entities.
real 0m50.007s
user 0m30.688s
sys 0m0.512s
Second run:
Ladsgroup added a comment.
First run on the props-repo.wmflabs.org without the patch:
ladsgroup@federated-wikis2:/var/www/html$ time php extensions/Wikibase/repo/maintenance/dumpRdf.php > /dev/null
Dumping shard 0/1
...
Processed 7034 entities.
real 0m40.751s
user 0m23.796s
sys 0m0.616s
Second
Ladsgroup added a comment.
First run on the items-repo with this patch:
ladsgroup@federated-wikis:/var/www/html$ time php extensions/Wikibase/repo/maintenance/dumpRdf.php > /dev/null
Dumping shard 0/1
...
real 0m32.192s
user 0m26.328s
sys 0m0.584s
Second run:
Ladsgroup added a comment.
First run on items-repo (items-repo.wmflabs.org) without this patch:
ladsgroup@federated-wikis:/var/www/html$ time php extensions/Wikibase/repo/maintenance/dumpRdf.php > /dev/null
Dumping shard 0/1
...
Processed 7042 entities.
real 0m30.012s
user 0m23.724s
sys 0m0.788s
Smalyshev added a comment.
Did this performance test happen?TASK DETAILhttps://phabricator.wikimedia.org/T162371EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: hoo, SmalyshevCc: Lucas_Werkmeister_WMDE, Smalyshev, daniel, WMDE-leszek, Aklapper,
27 matches
Mail list logo
