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

code for consideration (performance improvement) #39

Closed
epawel opened this Issue Jan 14, 2013 · 4 comments

Comments

Projects
None yet
4 participants
@epawel

epawel commented Jan 14, 2013

Some time ago the plugin stopped sending emails (also preview wasn't showing up). We send about 5000 emails (we have 500+ groups).
After an investigation I found that in_array was causing fatal error (time out) so I changed it. It seems to be faster and work fine (about 6 months) on our website. I hope it could be useful for others.

file: bp-activity-subscription-digest.php
function ass_digest_fire( $type )

now it's:

if ( !in_array( $sid, $all_activity_items ) ) {
$all_activity_items[] = $sid;
};

my version:

foreach ($all_activity_items as $key => $value) {
if($sid == $value){
$all_activity_items[] = $sid;
}
}

@dwenaus

This comment has been minimized.

Show comment
Hide comment
@dwenaus

dwenaus Jan 14, 2013

Collaborator

just guessing here, but this might do the trick, to cast the variable as an array before doing in_array. probably an isset check for both variables would be prudent.
if ( !in_array( $sid, (array) $all_activity_items ) ) {
$all_activity_items[] = $sid;
};

Collaborator

dwenaus commented Jan 14, 2013

just guessing here, but this might do the trick, to cast the variable as an array before doing in_array. probably an isset check for both variables would be prudent.
if ( !in_array( $sid, (array) $all_activity_items ) ) {
$all_activity_items[] = $sid;
};

@r-a-y

This comment has been minimized.

Show comment
Hide comment
@r-a-y

r-a-y Jan 14, 2013

Collaborator

@dwenaus' suggestion would make sense if an error popped up in the server error log.

I think @epawel might be on to something because in_array() is not efficient when there is a huge array, which is likely the case here. Perhaps we can use array_flip() and do an isset() check instead.

Collaborator

r-a-y commented Jan 14, 2013

@dwenaus' suggestion would make sense if an error popped up in the server error log.

I think @epawel might be on to something because in_array() is not efficient when there is a huge array, which is likely the case here. Perhaps we can use array_flip() and do an isset() check instead.

@boonebgorges

This comment has been minimized.

Show comment
Hide comment
@boonebgorges

boonebgorges Jan 14, 2013

Owner

+1 to what @r-a-y said. isset() is very fast, but I think
array_flip() has some overhead that might make @epawel's suggestion
more practical.

On 01/14/2013 04:18 PM, r-a-y wrote:

@dwenaus https://github.com/dwenaus dwenaus' suggestion would make
sense if an error popped up in the server error log.

I think @epawel https://github.com/epawel might be on to something
because |in_array()| is not efficient when there is a huge array, which
is likely the case here. Perhaps we can use |array_flip()| instead and
do an |isset()| check instead.


Reply to this email directly or view it on GitHub
#39 (comment).

Owner

boonebgorges commented Jan 14, 2013

+1 to what @r-a-y said. isset() is very fast, but I think
array_flip() has some overhead that might make @epawel's suggestion
more practical.

On 01/14/2013 04:18 PM, r-a-y wrote:

@dwenaus https://github.com/dwenaus dwenaus' suggestion would make
sense if an error popped up in the server error log.

I think @epawel https://github.com/epawel might be on to something
because |in_array()| is not efficient when there is a huge array, which
is likely the case here. Perhaps we can use |array_flip()| instead and
do an |isset()| check instead.


Reply to this email directly or view it on GitHub
#39 (comment).

r-a-y added a commit that referenced this issue Apr 29, 2013

Do not use in_array() when grabbing all activity IDs for digest users.
Instead use an isset() check and use array_keys() to reassemble
$all_activity_items array.

See #39.

r-a-y added a commit that referenced this issue Apr 29, 2013

Do not use in_array() when grabbing all activity IDs for digest users.
Instead use an isset() check and use array_keys() to reassemble
$all_activity_items array.

See #39.
@r-a-y

This comment has been minimized.

Show comment
Hide comment
@r-a-y

r-a-y May 15, 2013

Collaborator

Closing this one for now. Please reopen or create a new issue if there are any further improvements.

Collaborator

r-a-y commented May 15, 2013

Closing this one for now. Please reopen or create a new issue if there are any further improvements.

@r-a-y r-a-y closed this May 15, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment