Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some performance improvements for the all websites dashboard #7162

Merged
merged 5 commits into from Feb 11, 2015
Merged

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Feb 10, 2015

refs #6809

I tested it with 4k websites. The change makes it only a couple of seconds faster
and requesting 4k websites would still take 4-8 seconds even if only 10 sites of
them are actually requested. For example before we did request each site 3 times
(12k times in total and triggered 12k events etc) which should be no longer the case.
The last clearSiteCache was there to free some memory but I think it shouldn't
be problematic to remove it. Also I removed one 'sort' as a lot of time is wasted
there. It should be in theory sorted afterwards anyway again. Need to see test
results whether this change is good or not.

This doesn't fix the issue #6809, it is only one part of it.

There might be a failing test but this one is also failing on master.

…report.

I tested it with 4k websites. The change made it only a couple of seconds faster
but requesting 4k websites would still take 4-8 seconds even if only 10 sites of
them are actually requested. For example before we did request each site 3 times
(12k times in total and triggered 12k events etc) which should be no longer the case.
The last `clearSiteCache` was there to free some memory but I think it shouldn't
be problematic to remove it. Also I removed one 'sort' as a lot of time is wasted
there. It should be in theory sorted afterwards anyway again. Need to see test
results whether this change is good or not.
…ts in only one requested site but uses multi period
@tsteur
Copy link
Member Author

tsteur commented Feb 10, 2015

There is a failing test re test_BackwardsCompatibility1XTest__MultiSites.getAll_day.xml but I can't find that file. Neither in processed nor expected nor somewhere else. Don't know how I am supposed to fix this one.

@mattab
Copy link
Member

mattab commented Feb 10, 2015

the expected files are re-used from the test OneVisitorTwoVisits
this is done via the magic 'compareAgainst' => 'OneVisitorTwoVisits',in the test

@mattab mattab added this to the Piwik 2.11.0 milestone Feb 10, 2015
@tsteur
Copy link
Member Author

tsteur commented Feb 10, 2015

There's a bug in the output of the path then...

That's problematic since they have apparently different formats. Eg OneVisitor returns a different order than BC1xTest. Will have a look tmrw at this. Don't wanna deal with this today, already enough... ;)

@tsteur
Copy link
Member Author

tsteur commented Feb 10, 2015

I think the tests should be fixed now

@mattab mattab added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. and removed not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Feb 11, 2015
mattab pushed a commit that referenced this pull request Feb 11, 2015
Some performance improvements for the all websites dashboard
@mattab mattab merged commit 75e1cf1 into master Feb 11, 2015
@mattab
Copy link
Member

mattab commented Feb 11, 2015

Let's remember to test this on demo2 and demo to be safe @tsteur (i'll do new beta tomorrow)

@mattab
Copy link
Member

mattab commented Feb 11, 2015

and nice to see faster All Websites dashboard, step by step 👍

@tsteur tsteur deleted the 6809 branch March 8, 2015 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants