daniel-zahariev / php-aws-ses

PHP classes that interfaces Amazon Simple Email Service
307 stars 100 forks source link

Bulk Messages with attachments - performance #50

Closed AndriusCTR closed 6 years ago

AndriusCTR commented 6 years ago

Hello,

When bulk mode is used, per your example, every time the $ses->sendEmail($message); is called, the base64_encode is called for the same file attachment, for each iteration, over and over again? Is this true?

I think it is much better to resue the same base64_encoded string rather than clear it and build again thousand times when sending bulk email with attachment...

https://github.com/daniel-zahariev/php-aws-ses/blob/6b69aef6a2a778320d208b6288a58ac325ca6100/src/SimpleEmailServiceMessage.php#L366

Unless there is another way to build and send bulk email?

$ses->setBulkMode(true);

foreach($messages as $message) {
    $ses->sendEmail($message);
}

$ses->setBulkMode(false);

Thanks

daniel-zahariev commented 6 years ago

Thanks @AndriusCTR, this is now fixed with the latest release (0.9.1)

AndriusCTR commented 6 years ago

Thanks @daniel-zahariev for update, but looking at the code I am not sure it will solve the problem...

Correct me if I am wrong, but at the moment it will cache the $raw_message only if it doesn't change between iterations (inside bulk sending loop)? But when you are sending 'the same' email to many recipients, one thing definitely changes in the raw message - the "to" header (string).... so current cache won't be used at all during bulk mail sending?

I think the v.0.9.0 should have been modified to cache only the encoded attachment string, into local property, and then reuse it everytime the $ses->sendEmail($message) is called. So $raw_messages changes (because To: changes), but attachments are the same, so they are reused...

Another question: the attachment data is base64_encoded, and then the whole raw message (with the already encoded attachment data inside it) is base64_encoded again. So the attachment part is encoded twice? Is this the way it should be by the spec? I don't know myself, just wondering... Thanks!

daniel-zahariev commented 6 years ago

Thanks, will think about it over the weekend

AndriusCTR commented 6 years ago

At the moment I am sending bulk email to each recipient separately, like this:

$ses = new SimpleEmailService('AccessKey', 'SecretKey');
$ses->setBulkMode(true);

$recipients = ['email1@email1.com', 'email2@email2.com'];

$m = new SimpleEmailServiceMessage();
$m->addAttachmentFromFile('my_PFD_file.pdf', '/path/to/pdf/file', 'application/pdf');

foreach($recipients as $email)
{
    $m->addTo($email);

    $ses->sendEmail($m);

    $m->clearTo();
}

$ses->setBulkMode(false);

Maybe there is another way to do that (hard to tell from the readme), but this might help you to understand whole picture better. Thanks and will be waiting for your updates.