@tsteur opened this Pull Request on July 28th 2015 Member

Currently, a Row's columns are accessed like this: $this->c[self::COLUMNS][$columnName]. When archiving a big year instance we spend about 900 seconds (33%) just in getColumn(). By changing this to $this->columns[$columnName] the time dropped to 450 seconds (17%). By using ArrayObject the time drops by another 250 seconds. A difference in speed is not really noticeable in normal reports but when archiving aggregated reports (week, month, year, ranges).

Original PR was #7570

@mattab commented on July 30th 2015 Member

fyi: there is some code missing in this PR, eg. all unit tests were added inyour original PR https://github.com/piwik/piwik/pull/7570/files

@tsteur commented on July 30th 2015 Member

Not quite sure what you mean, tests were already added in another PR that was merged a while ago.

Also see comment in #7570:

Please note in this PR there are also some changes to subtableId etc. which will be probably in master soon anyway. I didn't make the effort to split those changes as we only wanted to still have the code in general in case those bugs will be fixed in Xdebug. It wasn't easy to implement it based on ArrayObject since it implements the Serializable interface and we need to make sure to stay BC with already serialized Row instances in a different format see code.

We implemented many changes re Serializable etc in another PR so here it is only about switching to ArrayObject and instead of using $this->columns[$name] -> $this[$name]

@tsteur commented on July 30th 2015 Member

I presume you got maybe confused with https://github.com/piwik/piwik/pull/8449 ?

@mattab commented on August 12th 2015 Member

I indeed got confused - LGTM :+1:

This Pull Request was closed on August 12th 2015
Powered by GitHub Issue Mirror