On Thu, 25 Jun 2009, Giuseppe Bilotta wrote:
> Views which contain many occurrences of the same email address (e.g.
> shortlog view) benefit from not having to recalculate the MD5 of the
> email address every time.
It would be nice to have some benchmarks comparing performance before
and after this patch.
>
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx>
Acked-by: Jakub Narebski <jnareb@xxxxxxxxx>
> ---
> gitweb/gitweb.perl | 24 ++++++++++++++++++++++--
> 1 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index f2e0cfe..d3bc849 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3257,6 +3257,27 @@ sub git_print_header_div {
> "\n</div>\n";
> }
>
> +# Rather than recomputing the url for an email multiple times, we cache it
> +# after the first hit. This gives a visible benefit in views where the avatar
> +# for the same email is used repeatedly (e.g. shortlog).
> +# The cache is shared by all avatar engines (currently gravatar only), which
> +# are free to use it as preferred. Since only one avatar engine is used for any
> +# given page, there's no risk for cache conflicts.
> +our %avatar_cache = ();
Nice explanation.
> +
> +# Compute the gravatar url for a given email, if it's not in the cache already.
> +# Gravatar stores only the part of the URL before the size, since that's the
> +# one computationally more expensive. This also allows reuse of the cache for
> +# different sizes (for this particular engine).
> +sub gravatar_url {
> + my $email = lc shift;
> + my $size = shift;
> + $avatar_cache{$email} ||=
> + "http://www.gravatar.com/avatar.php?gravatar_id="; .
> + Digest::MD5::md5_hex($email) . "&size=";
> + return $avatar_cache{$email} . $size;
> +}
Nice solution. Very good.
I guess it is not worth it to _not_ use cache for few avatars views
such as 'commit', 'commitdiff', in the future also 'tag' view, isn't it?
BTW. http://www.gravatar.com/site/implement/url recommends
http://www.gravatar.com/avatar/3b3be63a4c2a439b013787725dfce802 rather than
http://www.gravatar.com/avatar.php?gravatar_id=3b3be63a4c2a439b013787725dfce802
you use, following http://www.gravatar.com/site/implement/perl
Hmmm...
> +
> # Insert an avatar for the given $email at the given $size if the feature
> # is enabled.
> sub git_get_avatar {
> @@ -3266,8 +3287,7 @@ sub git_get_avatar {
> my $size = $avatar_size{$params{'size'}} || $avatar_size{'default'};
> my $url = "";
> if ($git_avatar eq 'gravatar') {
> - $url = "http://www.gravatar.com/avatar.php?gravatar_id="; .
> - Digest::MD5::md5_hex(lc $email) . "&size=$size";
> + $url = gravatar_url($email, $size);
> }
> # Currently only gravatars are supported, but other forms such as
> # picons can be added by putting an else up here and defining $url
Very nice.
> --
> 1.6.3.rc1.192.gdbfcb
>
>
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html