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

Speed up quoted printable encoding #228

Merged
merged 1 commit into from
Nov 18, 2020

Conversation

bossek
Copy link
Contributor

@bossek bossek commented Nov 17, 2020

Encoding to quoted printable was quite slow in cases when calling
string:substr and subsequent lists:concat for inserting soft newline was
used too often. New implementation uses only fast head operations.

Encoding to quoted printable was quite slow in cases when calling
string:substr and subsequent lists:concat for inserting soft newline was
used too often. New implementation uses only fast head operations.
@bossek
Copy link
Contributor Author

bossek commented Nov 17, 2020

For comparing before/after performance I used following:

Mail = fun(Body) ->
 {<<"multipart">>, <<"alternative">>,
  [{<<"From">>, <<"test@example.org">>}, {<<"To">>, <<"test@example.org">>}, {<<"Subject">>, <<"test">>}, {<<"Mime-Version">>, <<"1.0">>}], #{},
  [{"text", "html", [{<<"Content-Type">>, <<"text/html; charset=\"utf-8\"">>}, {<<"Content-Transfer-Encoding">>, <<"quoted-printable">>}],
  #{content_type_params => [{<<"charset">>, <<"utf-8">>}], disposition => <<"inline">>}, Body}]}
end.

timer:tc(fun() -> mimemail:encode(Mail(unicode:characters_to_binary(string:copies(" ř", 100000))), []) end).
timer:tc(fun() -> mimemail:encode(Mail(binary:list_to_bin(string:copies("a a", 100000))), []) end).

Both calls takes seconds before improvement and hundreds/tens of milliseconds after.

Copy link
Collaborator

@seriyps seriyps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

insert_soft_newline([H | T], AfterPos) when AfterPos > 0 ->
[H | insert_soft_newline(T, AfterPos - 1)];
insert_soft_newline(Str, 0) ->
[$\n, $\r, $= | Str].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: I think you can do "\n\r=" ++ Str and compiler will be able to optimize it to what your code looks like now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right! Should I change it before merge or keep it as it is?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine as it is now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks!

@mworrell
Copy link
Collaborator

Good work, thank you!

@mworrell
Copy link
Collaborator

@seriyps Ready to be merged?

@seriyps
Copy link
Collaborator

seriyps commented Nov 17, 2020

@mworrell yes

@mworrell
Copy link
Collaborator

@bossek Merging now, thank you for your contribution!

@mworrell mworrell merged commit f40cf02 into gen-smtp:master Nov 18, 2020
@bossek
Copy link
Contributor Author

bossek commented Nov 19, 2020

@mworrell Glad to help :-)

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

Successfully merging this pull request may close these issues.

3 participants