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

Improve mail sending by reduce MIME encoding process #1824

Open
changyy opened this issue Sep 5, 2019 · 9 comments
Open

Improve mail sending by reduce MIME encoding process #1824

changyy opened this issue Sep 5, 2019 · 9 comments

Comments

@changyy
Copy link

changyy commented Sep 5, 2019

Problem description

When you want to send small changes mail content to a lot of receivers one by one, the same attachments would be MIME encoded repeatedly.

It would cost CPU time/credit on AWS, so I try to propose a solution.

Code to reproduce

Look at sample code: https://github.com/PHPMailer/PHPMailer/blob/master/examples/smtp.phps

Use a while loop to reproduce situation:

foreach( [ /* .. */ ] as $receiver ) {
    //Create a new PHPMailer instance
    $mail = new PHPMailer;
    // ...
    $mail->addAddress($receiver);
    // ...
    $mail->Subject = 'Hi '.$receiver;
    // ...
    $mail->addAttachment('images/phpmailer_mini.png');
    // ...
}

Debug output

None

Solution:

Providing a MIME Encoding Cache Store to handle it.

changyy added a commit to changyy/PHPMailer that referenced this issue Sep 5, 2019
@Synchro
Copy link
Member

Synchro commented Sep 5, 2019

I get what you're trying to do here, however, the implementation you wrote has some fairly major issues - it represents the only time that PHPMailer would attempt to write to disk, preventing it from working in read-only environments. It would be better to keep the cache in memory, or better, make it pluggable with a PSR-6 instance.

That said, it's not clear whether this brings any significant performance improvement since instead of reading a file once, it would need to read it twice and write it once. Do you have benchmarks to back it up?

Your usage example in this ticket is already sub-optimal because it creates a new instance inside the loop, and there can be no benefit at all in the example file because it only sends one message - it also strikes me as unnecessary to add a complete example purely to add one line - if we're going to have a cache like this, it should probably be automatic and not require user code changes.

Overall, what I'd really like to see to improve attachment handling is to remove the need to store attachments in memory at all - have them encoded dynamically using a generator, which would be faster and much more memory efficient. I suspect this would require a BC break and a PHP version bump, which I'm considering for future versions anyway.

changyy added a commit to changyy/PHPMailer that referenced this issue Sep 5, 2019
@changyy
Copy link
Author

changyy commented Sep 5, 2019

Hi Synchro,

Thanks for your review.

This solution could be running at Read-Only file system because using PHP array type to keep the MIME encoding result in memory.

//
// $this->ReduceMIMEEncodeCacheStore = [];
//
if (is_array($this->ReduceMIMEEncodeCacheStore)) {
	$cache_key = "$path\t$encoding";
	if (isset($this->ReduceMIMEEncodeCacheStore[$cache_key])) {
		$file_buffer = $this->ReduceMIMEEncodeCacheStore[$cache_key];
		$this->edebug('encodeFile: use cache ['.$path.']['.$encoding.']');
	} else {
		$file_buffer = file_get_contents($path);
		if (false === $file_buffer) {
			throw new Exception($this->lang('file_open') . $path, self::STOP_CONTINUE);
		}
		$file_buffer = $this->encodeString($file_buffer, $encoding);
		$this->ReduceMIMEEncodeCacheStore[$cache_key] = $file_buffer;
		$this->edebug('encodeFile: set cache ['.$path.']['.$encoding.']');
	}
} 

Performance improvement:

$ php -v
PHP 7.1.23 (cli) (built: Feb 22 2019 22:19:32) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2018 Zend Technologies
$ git clone https://github.com/changyy/PHPMailer.git
$ cd PHPMailer
$ php ~/bin/composer.phar install --no-dev
$ cd examples
$ php examples/smtp_reduce_mime_encoding.phps 
$ php smtp_reduce_mime_encoding.phps 
Genrate 1000 receivers: 1000
Sending 1000 mails with creating a new PHPMailer instance: 0.41333603858948 sec
Sending 1000 mails with only one PHPMailer instance: 1.745913028717 sec
Sending 1000 mails with creating a new PHPMailer instance and ReduceMIMEEncodeCacheStore: 0.32201600074768 sec
Sending 1000 mails with only one PHPMailer instance and ReduceMIMEEncodeCacheStore: 1.6159191131592 sec

I think using PHP array type to provide the key-value cache to reduce MIME encoding will reduce CPU usage.

@Synchro
Copy link
Member

Synchro commented Sep 5, 2019

You have some very strange results there that are hard to explain - using a single PHPMailer instance means you can use keepalive which eliminates TLS handshakes and multiple SMTP commands, so you have to be doing something odd for that to be slower. Can you share your benchmark code?

@changyy
Copy link
Author

changyy commented Sep 5, 2019

Hi Synchro,

Code: https://github.com/changyy/PHPMailer/blob/master/examples/smtp_reduce_mime_encoding.phps

Those processes are only on building a mail content.

    //Build the mail content
    $mail->preSend(); 

changyy added a commit to changyy/PHPMailer that referenced this issue Sep 5, 2019
changyy added a commit to changyy/PHPMailer that referenced this issue Sep 5, 2019
@Synchro
Copy link
Member

Synchro commented Sep 5, 2019

I've tried this locally, changing three things

  • allowing actual sending to a local fake SMTP server
  • enabling keepalive when using a single PHPMailer instance
  • reduced the send count for sanity!
Genrate 200 receivers: 200
Sending 200 mails with creating a new PHPMailer instance: 7.8507869243622 sec
Sending 200 mails with only one PHPMailer instance: 5.2449159622192 sec
Sending 200 mails with creating a new PHPMailer instance and ReduceMIMEEncodeCacheStore: 9.2903461456299 sec
Sending 200 mails with only one PHPMailer instance and ReduceMIMEEncodeCacheStore: 5.9140291213989 sec

So the cache seems to make it go slower?

@changyy
Copy link
Author

changyy commented Sep 5, 2019

Hi Synchro,

This result is very interesting. :D

My simple cache design is to speed up the processing function encodeFile($path, $encoding = self::ENCODING_BASE64) by substituting memory space for computing power. It would be used at $mail->addAttachment('images/phpmailer_mini.png');.

Would you mind to take a try to get the same result with a larger image attachment? Image like 50KB, 100KB size. The EDM is usually with 100 KB~400 KB size.

In my work, I need to send 100,000 emails with same
Email subject and image attachment. The mail content would be changed based on the recipient name and email address of each message and using PHPMailer to send the mails one by one. I still think sending a mail with take an Image file MIME encoding per run would slower then lookup the encoded Image content from memory (PHP Array).

Maybe I need to study PHPMailer more deeper to change my mind. :)

Thanks for your review.

changyy added a commit to changyy/PHPMailer that referenced this issue Sep 6, 2019
$ php benchmark_reduce_mime_encoding.phps
...
Size    Method1 Method2 Method3 Method4
1.8kb   0.0600  0.1133  0.0553  0.0974
5.7kb   0.0700  0.1526  0.0598  0.1359
50kb    0.0786  0.1687  0.0592  0.1386
100kb   0.0897  0.1782  0.0619  0.1439
200kb   0.1159  0.2045  0.0662  0.1470
500kb   0.1930  0.2837  0.0707  0.1546
changyy added a commit to changyy/PHPMailer that referenced this issue Sep 6, 2019
$ php benchmark_reduce_mime_encoding.phps
...
Size	Method1	Method2	Method3	Method4
1.8kb	0.0579	0.1069	0.0515	0.1021
5.7kb	0.0682	0.1526	0.0561	0.1862
50kb	0.0829	0.1543	0.0571	0.1555
100kb	0.0938	0.1646	0.0587	0.1469
200kb	0.1282	0.1895	0.0803	0.1567
500kb	0.1888	0.2793	0.0742	0.1686

Method1: Send mails with creating a new PHPMailer instance.
Method2: Send mails with only one PHPMailer instance.
Method3: Send mails with creating a new PHPMailer instance and ReduceMIMEEncodeCacheStore.
Method4: Send mails with only one PHPMailer instance and ReduceMIMEEncodeCacheStore.
changyy added a commit to changyy/PHPMailer that referenced this issue Sep 6, 2019
$ php benchmark_reduce_mime_encoding.phps
...
Size	Method1	Method2	Method3	Method4
1.8kb	0.0579	0.1069	0.0515	0.1021
5.7kb	0.0682	0.1526	0.0561	0.1862
50kb	0.0829	0.1543	0.0571	0.1555
100kb	0.0938	0.1646	0.0587	0.1469
200kb	0.1282	0.1895	0.0803	0.1567
500kb	0.1888	0.2793	0.0742	0.1686

Method1: Send mails with creating a new PHPMailer instance.
Method2: Send mails with only one PHPMailer instance.
Method3: Send mails with creating a new PHPMailer instance and ReduceMIMEEncodeCacheStore.
Method4: Send mails with only one PHPMailer instance and ReduceMIMEEncodeCacheStore.
@changyy
Copy link
Author

changyy commented Sep 6, 2019

I've tried this locally, changing three things

  • allowing actual sending to a local fake SMTP server
  • enabling keepalive when using a single PHPMailer instance
  • reduced the send count for sanity!
Genrate 200 receivers: 200
Sending 200 mails with creating a new PHPMailer instance: 7.8507869243622 sec
Sending 200 mails with only one PHPMailer instance: 5.2449159622192 sec
Sending 200 mails with creating a new PHPMailer instance and ReduceMIMEEncodeCacheStore: 9.2903461456299 sec
Sending 200 mails with only one PHPMailer instance and ReduceMIMEEncodeCacheStore: 5.9140291213989 sec

So the cache seems to make it go slower?

I found an improved way: Skip file_exists checking when using ReduceMIMEEncodeCacheStore designed.

You may take a try by using this version code, https://github.com/changyy/PHPMailer/blob/master/src/PHPMailer.php, to run your benchmark.

I also run the benchmark on multiple file size:

https://github.com/changyy/PHPMailer/blob/master/examples/benchmark_reduce_mime_encoding.phps

Size	Method1	Method2	Method3	Method4
1.8kb	0.0579	0.1069	0.0515	0.1021
5.7kb	0.0682	0.1526	0.0561	0.1862
50kb	0.0829	0.1543	0.0571	0.1555
100kb	0.0938	0.1646	0.0587	0.1469
200kb	0.1282	0.1895	0.0803	0.1567
500kb	0.1888	0.2793	0.0742	0.1686

Method1: Send mails with creating a new PHPMailer instance.
Method2: Send mails with only one PHPMailer instance.
Method3: Send mails with creating a new PHPMailer instance and ReduceMIMEEncodeCacheStore.
Method4: Send mails with only one PHPMailer instance and ReduceMIMEEncodeCacheStore.

I think the ReduceMIMEEncodeCacheStore solution will be faster when the total size of the attachments exceeds 100KB.

@Synchro
Copy link
Member

Synchro commented Sep 6, 2019

I really recommend you use a PSR-6 or PSR-16 interface for this and allow users to inject a cache handler, even something simple like this, or any of these adapters. It will eliminate nearly all of your code and allow users to choose a cache implementation (e.g. disk / array / memcache / redis) without PHPMailer having to be concerned about the details. It might also allow the cache to be used for other things, perhaps punycoded addresses, for example.

I do think it's strange that you're getting slower results when using a single instance - doing that runs much less code and uses less memory, so I don't know why it would be slower - an xdebug profile would show what's happening.

Using the external array by reference is effectively equivalent to having an instance property that's reused across multiple sends (i.e. what a single instance would do).

One minor thing - ReduceMIMEEncodeCacheStore is an unnecessarily long and confusing name - something simpler like MIMECache, or even just Cache would be more understandable, especially if it might be used for other things.

changyy added a commit to changyy/PHPMailer that referenced this issue Sep 6, 2019
$ php benchmark_reduce_mime_encoding.phps
...
Size	Method1	Method2	Method3	Method4
1.8kb	0.0646	0.1114	0.0561	0.1054
5.7kb	0.0757	0.1610	0.0624	0.1452
50kb	0.0846	0.1856	0.0623	0.1469
100kb	0.0989	0.1862	0.0740	0.1496
200kb	0.0636	0.1117	0.0563	0.1058
500kb	0.2195	0.3179	0.0834	0.1745

Method1: Send mails with creating a new PHPMailer instance.
Method2: Send mails with only one PHPMailer instance.
Method3: Send mails with creating a new PHPMailer instance and ReduceMIMEEncodeCacheStore.
Method4: Send mails with only one PHPMailer instance and ReduceMIMEEncodeCacheStore.
changyy added a commit to changyy/PHPMailer that referenced this issue Sep 6, 2019
$ php benchmark_reduce_mime_encoding.phps
...
Size	Method1	Method2	Method3	Method4
1.8kb	0.0646	0.1114	0.0561	0.1054
5.7kb	0.0757	0.1610	0.0624	0.1452
50kb	0.0846	0.1856	0.0623	0.1469
100kb	0.0989	0.1862	0.0740	0.1496
200kb	0.0636	0.1117	0.0563	0.1058
500kb	0.2195	0.3179	0.0834	0.1745

Method1: Send mails with creating a new PHPMailer instance.
Method2: Send mails with only one PHPMailer instance.
Method3: Send mails with creating a new PHPMailer instance and ReduceMIMEEncodeCacheStore.
Method4: Send mails with only one PHPMailer instance and ReduceMIMEEncodeCacheStore.
changyy added a commit to changyy/PHPMailer that referenced this issue Sep 7, 2019
Using psr-16-simple-cache on MIMECache

$ php composer require psr/simple-cache

$ php benchmark_reduce_mime_encoding.phps
...
Size	Method1	Method2	Method3	Method4
1.8kb	0.0645	0.1205	0.0573	0.1064
5.7kb	0.0814	0.1740	0.0605	0.1509
50kb	0.0872	0.1698	0.0622	0.1473
100kb	0.1030	0.1839	0.0678	0.1496
200kb	0.1302	0.2208	0.0677	0.1567
500kb	0.2261	0.3194	0.0878	0.1730
@changyy
Copy link
Author

changyy commented Sep 7, 2019

Hi Synchro,

I have implemented it on PSR-16 SimpleCache interface and change the field name from ReduceMIMEEncodeCacheStore to MIMECache.

Something changed:

  • composer.json
    "require": {
        "php": ">=5.5.0",
        "ext-ctype": "*",
        "ext-filter": "*",
        "psr/simple-cache": "^1.0"
    },

The benchmark:

    Using psr-16-simple-cache on MIMECache
    
    $ php composer require psr/simple-cache
    
    $ php benchmark_mime_cache.phps
    ...
    Size    Method1 Method2 Method3 Method4
    1.8kb   0.0645  0.1205  0.0573  0.1064
    5.7kb   0.0814  0.1740  0.0605  0.1509
    50kb    0.0872  0.1698  0.0622  0.1473
    100kb   0.1030  0.1839  0.0678  0.1496
    200kb   0.1302  0.2208  0.0677  0.1567
    500kb   0.2261  0.3194  0.0878  0.1730

    Method1: Send mails with creating a new PHPMailer instance.
    Method2: Send mails with only one PHPMailer instance.
    Method3: Send mails with creating a new PHPMailer instance and MIMECache.
    Method4: Send mails with only one PHPMailer instance and MIMECache.

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

No branches or pull requests

2 participants