@tolbon opened this Pull Request on May 26th 2020 Contributor

TAKE WITH CAUTION.

I inspect what function have lot of own time in matomo
I see this :
Screenshot from 2020-05-26 12-03-00
After some modification "own time" decreased (the 3 columns, time in ms)
Screenshot from 2020-05-26 12-02-18

cause some functions like count, is_array, is_string ... if you add \ before PHP optimize this call.

the bench is made in this page. I refresh and aggregates 8 cachegrind files
Screenshot from 2020-05-26 12-07-32

@tolbon commented on May 26th 2020 Contributor

Can you validate this optimization ? I am not sure with measure.

@tsteur commented on May 26th 2020 Member

@tolbon could you also let us know in the cachegrind what the column name is for each number? We have been doing quite a few profiles in the past and these changes shouldn't really have an impact on performance.

When doing the profiles make sure Matomo development mode is disabled and caches are warmed up for example. Also be good to make sure to test the profiles several times as they can vary sometimes quite a bit.

Is there any chance you can share the full cachegrind file? I suppose the file be quite large?

@tolbon commented on May 27th 2020 Contributor

@tolbon could you also let us know in the cachegrind what the column name is for each number?

Name, Time, Own Time, Memory, OwnMemory, NbCalls

We have been doing quite a few profiles in the past and these changes shouldn't really have an impact on performance.

I am not sure It's a PHP7+ optimization. maybe if you do this before explain what.

I am using matomo docker example

@tsteur commented on May 27th 2020 Member

BTW looking at the 2 screenshots again. Do you notice that the methods are called less often in the second run? Be great to profile the identical request several times and compare the several requests and to make sure there's nothing else running on the server for accurate results.

We've done a lot of profiling on PHP 7+ and from what we know this should make like absolutely no difference. Could be wrong though.

As mentioned with the profiling be great to ensure to compare the same request, run the profile several times cause there will be always some minor differences, check that nothing else is running on that server that might consume CPU for example.

If you can send us a full cachegrind file that be really interesting to look at

@tolbon commented on May 27th 2020 Contributor

Hello,
https://send.firefox.com/download/507f7da404b3e24b/#r0qyhY_ylNwaxAUX_MSTog
(72Mo)
V1 is original
V2 is Just this commit
V3 is PHP-CS-Fixer fix --rules=native_constant_invocation (Add leading \ before constant invocation of internal constant to speed up resolving)

I know V2 miss one file Don’t know why.

You work on it always. You have certainly right about no effect of this PR.

@tsteur commented on May 27th 2020 Member

Had a look at some and compared them. Quite interesting the result. To know how accurate the profiles are be great if you could confirm you tested this ideally on a server where nothing else is running, and ideally would need to run the same test quite often to basically end up with some averages. Eg if you run the same test with the same code again, you might notice also quite a difference each time you run it.

@tolbon commented on May 27th 2020 Contributor

Ok I see. that’s why I prefer you to test you or your team in your side with this modification.

Add slash not change code result but let php microOptimize.

@tolbon commented on May 29th 2020 Contributor

@tsteur
Just a question about code :
In Piwik/CacheId::pluginAware();
sometimes it's call like +1 000 times and call this md5(implode('', $pluginNames)).
I am not sure but I think md5(implode()) return same value +1 000 and recompute this and take 5Mo Ram.
If it's same result I don't know how to save result of this and stock them somewhere cause it's in static call. Maybe create a method in Piwik\Plugin\Manager.php.

Why I search that ? Cause in cachegrind files check memory consumption and I see this function.
I search why Common::getRequestVar take some RAM too

@tsteur commented on June 1st 2020 Member

If it's same result I don't know how to save result of this and stock them somewhere cause it's in static call. Maybe create a method in Piwik\Plugin\Manager.php.

We actually don't want to save the result as when this method is used, it's important to detect any change in plugins etc.

5MB memory doesn't sound too bad? Although it should technically free the memory each time. Not sure if you get to see peak memory usage etc.

Generally that method shouldn't cause much of an issue.

Be keen to learn more about your profiling. Are you having memory issues? or performance issues with Matomo? Cause the tracking requests should consume very little memory and the DB takes typically most of the time. In the UI the highest memory consumption usually comes from reading reports (unserialising all the rows, merging rows from reports when needed etc). Performance wise there's a bit that can be done here and there but it might be not that trivial to make it more than 10% faster.

BTW we're using here the tideways profiler, some use https://blackfire.io/

@tolbon commented on June 2nd 2020 Contributor

No no it's not a leak I just ask. it's like compute 2 + 2 = 4 (1000 time during a request) if the result is the same 1000 times I think it's bad to recompute it 999 times.

ok next time I go with blackfire profile :)

@tsteur commented on June 2nd 2020 Member

BTW... end of the year PHP 8 should be released which might come with quite some big performance boost without needing to do any change. It's obviously to be seen if there is a performance boost and how much.

@tolbon commented on June 2nd 2020 Contributor

Yeah very excited about PHP 8 + JIT.
I don’t see I close this PR 😅

This Pull Request was closed on June 2nd 2020
Powered by GitHub Issue Mirror