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

cpu used is greater than 90% #117

Closed
meoliver opened this issue Jan 4, 2017 · 17 comments
Closed

cpu used is greater than 90% #117

meoliver opened this issue Jan 4, 2017 · 17 comments
Labels

Comments

@meoliver
Copy link

meoliver commented Jan 4, 2017

hello,When the app is I use this, very high CPU。Especially hot lead to equipment。

This thing when I commented code, this phenomenon is gone.
ex:
//PiwikTracker
// [PiwikTracker sharedInstanceWithSiteID:PiwikProductionSiteId baseURL:[NSURL URLWithString:PiwikProductionServerURL]];
// [PiwikTracker sharedInstance].includeDefaultCustomVariable = NO;
// [PiwikTracker sharedInstance].dispatchInterval = 5;

@brototyp
Copy link
Member

brototyp commented Jan 4, 2017

Hi @meoliver, thanks for your report. Can you record an instrument session with the Time Profiler so we can debug which code is actually causing the load?

@brototyp brototyp added the bug label Jan 4, 2017
@meoliver
Copy link
Author

meoliver commented Jan 4, 2017

I have to reason, but some question, need you to explain.

When the request error ,call back,
class PiwikNSURLSessionDispatcher method shouldAbortdispatchForNetworkError
when return yes( error.code == NSURLErrorCannotFindHost )。
Then call Class PiwikTracker method sendEvent(failureBlock line: 1033),and hasMore is yes。
This time, the request will be continuous, leading to excessive CPU load。

Call Class PiwikTracker method eventsFromStore: completionBlock: (line 1490 ), completionBlock parameter hasMore may be yes。

I want to know Class PiwikTracker line1461 -- line 1490
fetchRequest.fetchLimit、returnCount、eventEntities.count ,Their calculation logic is to do?

thanks

@meoliver
Copy link
Author

meoliver commented Jan 6, 2017

@brototyp If you are paying attention to my question?

@brototyp
Copy link
Member

brototyp commented Jan 6, 2017

Hi @meoliver, thanks for digging into the issue. As far as I understand you: When there is an issue transmitting an event to the server and not all events can be transmitted with one call to the server, the SDK will try to transmit the same event(s) over and over. If the error is always the same it will always repeat to transmit the same event over and over again.

As far as I can see this should be prevented by the shouldContinue value in the failure completion block. The 'PiwikNSURLSessionDispatcher.m' has a '-shouldAbortdispatchForNetworkError:' method that should return yes for a NSURLErrorCannotFindHost error. This matches with what you found out. I am not sure why in your case this leads to excessive CPU load.

Did you were able to record an Instruments session with the Time Profiler?
Unfortunately I am not sure what exactly your question is. Can you rephrase it?

@meoliver
Copy link
Author

meoliver commented Jan 7, 2017

@brototyp thanks。

when '-shouldAbortdispatchForNetworkError:' return yes,and call 'eventsFromStore: completionBlock: '

When the core data, save the data after the article has more than 20.

numberOfEvents is 20;
method ’eventsFromStore:(NSUInteger)numberOfEvents completionBlock: ‘ {


completionBlock(entityIDs, events, eventEntities.count == fetchRequest.fetchLimit ? YES : NO);

// this ‘eventEntities.count == fetchRequest.fetchLimit‘ is yes。


}

NSURLErrorCannotFindHost error ,There has been。

method 'sendEvent '

void (^failureBlock)(BOOL shouldContinue) = ^ (BOOL shouldContinue) {
PiwikDebugLog(@"Failed to send stats to Piwik server");

    if (shouldContinue) {

******* this code pay attention to******
[weakSelf sendEventDidFinishHasMorePending:hasMore];

// method ’sendEventDidFinishHasMorePending:' hasMore is yes, call ’sendEvent‘,but NSURLErrorCannotFindHost error,He'll cycle call。

// ’sendEvent‘----’eventsFromStore:completionBlock:‘----failureBlock() ----- ’sendEvent‘

******* this code pay attention to******
} else {
[weakSelf sendEventDidFinishHasMorePending:NO];
}

  }

@brototyp
Copy link
Member

brototyp commented Jan 8, 2017

@meoliver I am still not sure what you are meaning. Let me try to elaborate:

  • PiwikTracker:sendEvent gets called
  • numberOfEventsToSend is less than existing events, so hasMore is true
  • the events are tried to be transmitted, but there is an NSURLErrorCannotFindHost error
  • the failure-block (PiwikTracker:1029) is called with a shouldContinue = false
  • because shouldContinue is false the sendEventDidFinishHasMorePending is called with hasMore = false
  • sendEventDidFinishHasMorePending restarts the timer and exits

Is this correct? Did you were able to record an Instruments session with the Time Profiler? Please attach a Time Profiler session.

@meoliver
Copy link
Author

meoliver commented Jan 9, 2017

@brototyp thanks。

[PiwikTracker sharedInstanceWithSiteID:PiwikProductionSiteId baseURL:[NSURL URLWithString:@"http://tracking.googlerror.com/piwik/"]];
[PiwikTracker sharedInstance].includeDefaultCustomVariable = NO;
[PiwikTracker sharedInstance].dispatchInterval = 5;

The @"http://tracking.googlerror.com/piwik/" is NSURLErrorCannotFindHost

the failure-block (PiwikTracker:1033) add nslog(),For example:

if (shouldContinue) {
NSLog(@"Piwik has been called");
[weakSelf sendEventDidFinishHasMorePending:hasMore];
} else {
[weakSelf sendEventDidFinishHasMorePending:NO];
}

and Click on the for more than 21 times。look the log

@meoliver
Copy link
Author

meoliver commented Jan 9, 2017

@brototyp Hi,this is time profiler. Thanks
piwik.zip

@brototyp
Copy link
Member

brototyp commented Jan 9, 2017

Hi @meoliver, thanks for the recording. I was able to reproduce the issue and I guess I know what the issue is: PiwikNSURLSessionDispatcher:shouldAbortdispatchForNetworkError retunes YES if dispatching should be aborted. This method is used in PiwikNSURLSessionDispatcher:97. But its value is used as a parameter for the faliure-blocks parameter shouldContinue. That is exactly the wrong thing. If I see it correct line 97 should be: failureBlock(![self shouldAbortdispatchForNetworkError:error]); so inverting the bool value. Can you try that and get back in touch with me?

@meoliver
Copy link
Author

meoliver commented Jan 10, 2017

@brototyp thanks
failureBlock(![self shouldAbortdispatchForNetworkError:error]);
I think it will not solve the problem。 when NSURLErrorTimedOut,Still can continuous to send the request。

failureBlock(NO);
This will avoid, but it's meaningless to parameters.

@meoliver
Copy link
Author

@brototyp Hi,
I want to know Class ‘PiwikTracker’ line1461 to line 1490
fetchRequest.fetchLimit、returnCount、eventEntities.count ,Their calculation logic is to do?

line 1490
completionBlock(entityIDs, events, eventEntities.count == fetchRequest.fetchLimit ? YES : NO);

The third parameter is yes,and shouldAbortdispatchForNetworkError: return yes 。This will call cycles。

thanks

@brototyp
Copy link
Member

I think that is expected behavior. The sdk continues trying to transmit events if there is a timeout. We expect it to just be a temporary issue. Do you know what your timeout duration is? Here the timeout duration decides the "looping". So if the timeout is 1s the sdk will loop once per second. Which is an issue in the architecture. Better would be: If an event can't be transmitted in a transmission run we should not retry this event in this run. That is a bigger issue and requires quite some refactoring so we only fetch events we haven't already fetched.

I am not sure what your question there is. The eventEntities.count == fetchRequest.fetchLimit ? YES : NO is meant to decide it there are (or might be) more events queued. There is an issue here, that if there are exactly the amount of events queued that we want to transmit this will try one more time but I guess thats fine.

@meoliver
Copy link
Author

@brototyp Thank you very much care about my question.
So, this code you will follow the new to GitHub?
failureBlock(![self shouldAbortdispatchForNetworkError:error]);

Or,I modify the native code?

@brototyp
Copy link
Member

Please try to change it locally in your code. We are currently rewriting the whole sdk, fully written in swift. We are making sure this bug doesn't exist in the new sdk.

@tkapler
Copy link

tkapler commented Mar 16, 2017

@meoliver - do you have a patch for this problem? We face it as well and we cannot wait for swift version. Could you please send it to me (tomas.kapler@etnetera.cz). Thank you.

@brototyp
Copy link
Member

@tkapler Have you tried the patch I proposed? If so, can you give us some feedback if it changed anything or if it did not affect the issue. I did interpret the silence @meoliver as approval that the patch works.

@brototyp
Copy link
Member

Closing this for now. Feel free to reopen if you have more information on this topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants