Re: svn commit: r1461869 - /httpd/httpd/trunk/server/util.c
* Rainer Jung wrote:
On 27.03.2013 22:57, jaillet...@apache.org wrote:
Author: jailletc36
Date: Wed Mar 27 21:57:44 2013
New Revision: 1461869
URL: http://svn.apache.org/r1461869
Log:
Be more clever when allocating memory for log item to be escaped.
This should save about 70-100 bytes in the request pool with the
default config.
Modified:
httpd/httpd/trunk/server/util.c
Modified: httpd/httpd/trunk/server/util.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=146186
9r1=1461868r2=1461869view=diff
===
=== --- httpd/httpd/trunk/server/util.c (original)
+++ httpd/httpd/trunk/server/util.c Wed Mar 27 21:57:44 2013
@@ -1850,12 +1850,26 @@ AP_DECLARE(char *) ap_escape_logitem(apr
char *ret;
unsigned char *d;
const unsigned char *s;
+int length = 0;
if (!str) {
return NULL;
}
-ret = apr_palloc(p, 4 * strlen(str) + 1); /* Be safe */
+/* First, compute the space needed for the escaped string.
+ * This could be tweaked a bit for '\b', '\n'... These characters
+ * should not be common, so do not bother. */
+s = (const unsigned char *)str;
+for (; *s; ++s) {
+if (TEST_CHAR(*s, T_ESCAPE_LOGITEM)) {
+length += 4;/* for '\\' + c2x() */
+}
+else {
+length++;
+}
+}
+
+ret = apr_palloc(p, length + 1);
d = (unsigned char *)ret;
Here we treat CPU against memory. AFAIK this function is in the hot code
path, because it is used for many of the fields used in the default
access log format and also in many custom ones. All other uses seem to
be either trace log messages, some (few) error log messages and
mod_status output. For those cases I think neither the CPU nor the
Memory differences are important.
In the access log case it seems the typical fields which are run through
ap_escape_logitem() should rarely contain characters to escape. So an
alternative strategy would be to simply copy the string if we don't find
a character to escape. The second check for each char is not necessary
then.
A quick draft patch is at
http://people.apache.org/~rjung/patches/ap_escape_logitem_enhanced.patch
+ret = apr_palloc(p, length + 3 * escapes);
It should be still 4 * escapes ('\xHH').
nd
--
Das, was ich nicht kenne, spielt stückzahlmäßig *keine* Rolle.
-- Helmut Schellong in dclc
Re: svn commit: r1461869 - /httpd/httpd/trunk/server/util.c
* André Malo wrote:
* Rainer Jung wrote:
A quick draft patch is at
http://people.apache.org/~rjung/patches/ap_escape_logitem_enhanced.patc
h
+ret = apr_palloc(p, length + 3 * escapes);
It should be still 4 * escapes ('\xHH').
Also, escapes should be apr_size_t as well, I think.
nd
--
s s^saoaaaoaaoaaaom a alataa aaoat a a
a maoaa a laoata a oia a o a m a o alaoooat aaool aaoaa
matooololaaatoto aaa o a o ms;s;\s;s;g;y;s;:;s;y#mailto: #
\51/\134\137| http://www.perlig.de #;print;# n...@perlig.de
RE: svn commit: r1461869 - /httpd/httpd/trunk/server/util.c
-Original Message-
From: André Malo [mailto:n...@perlig.de]
Sent: Mittwoch, 17. April 2013 11:04
To: dev@httpd.apache.org
Subject: Re: svn commit: r1461869 - /httpd/httpd/trunk/server/util.c
* Rainer Jung wrote:
http://people.apache.org/~rjung/patches/ap_escape_logitem_enhanced.patch
+ret = apr_palloc(p, length + 3 * escapes);
It should be still 4 * escapes ('\xHH').
I don't think so. If you have the binary data of \xHH it is ok to have one byte
for the original data
plus 3 bytes for the escapes and that is length + 3 * escapes. The storage for
the original data is in
length. But I admit that I got confused by that as well :-)
Regards
Rüdiger
Re: svn commit: r1461869 - /httpd/httpd/trunk/server/util.c
On 17.04.2013 11:07, André Malo wrote:
* André Malo wrote:
* Rainer Jung wrote:
A quick draft patch is at
http://people.apache.org/~rjung/patches/ap_escape_logitem_enhanced.patc
h
+ret = apr_palloc(p, length + 3 * escapes);
It should be still 4 * escapes ('\xHH').
Hmmm, isn't the 1 = 4-3 already counted in length? Here we only add the
escaping overhead of 3 bytes per escape.
Also, escapes should be apr_size_t as well, I think.
Probably.
Thanks,
Rainer
Re: svn commit: r1461869 - /httpd/httpd/trunk/server/util.c
On 17.04.2013 11:38, Plüm, Rüdiger, Vodafone Group wrote:
-Original Message-
From: André Malo [mailto:n...@perlig.de]
Sent: Mittwoch, 17. April 2013 11:04
To: dev@httpd.apache.org
Subject: Re: svn commit: r1461869 - /httpd/httpd/trunk/server/util.c
* Rainer Jung wrote:
http://people.apache.org/~rjung/patches/ap_escape_logitem_enhanced.patch
+ret = apr_palloc(p, length + 3 * escapes);
It should be still 4 * escapes ('\xHH').
I don't think so. If you have the binary data of \xHH it is ok to have one
byte for the original data
plus 3 bytes for the escapes and that is length + 3 * escapes. The storage
for the original data is in
length. But I admit that I got confused by that as well :-)
So probably we should use a better comment as the one in the patch
currently:
/* 3 * escapes for the overhead in '\\' + c2x() */
;)
Rainer
Re: svn commit: r1461869 - /httpd/httpd/trunk/server/util.c
* Rainer Jung wrote:
On 17.04.2013 11:38, Plüm, Rüdiger, Vodafone Group wrote:
-Original Message-
From: André Malo [mailto:n...@perlig.de]
Sent: Mittwoch, 17. April 2013 11:04
To: dev@httpd.apache.org
Subject: Re: svn commit: r1461869 - /httpd/httpd/trunk/server/util.c
* Rainer Jung wrote:
http://people.apache.org/~rjung/patches/ap_escape_logitem_enhanced.pat
ch
+ret = apr_palloc(p, length + 3 * escapes);
It should be still 4 * escapes ('\xHH').
I don't think so. If you have the binary data of \xHH it is ok to have
one byte for the original data plus 3 bytes for the escapes and that is
length + 3 * escapes. The storage for the original data is in length.
But I admit that I got confused by that as well :-)
O. right.
So probably we should use a better comment as the one in the patch
currently:
/* 3 * escapes for the overhead in '\\' + c2x() */
;)
good idea ;)
nd
--
Solides und umfangreiches Buch
-- aus einer Rezension
http://pub.perlig.de/books.html#apache2
Re: svn commit: r1461869 - /httpd/httpd/trunk/server/util.c
On 27.03.2013 22:57, jaillet...@apache.org wrote:
Author: jailletc36
Date: Wed Mar 27 21:57:44 2013
New Revision: 1461869
URL: http://svn.apache.org/r1461869
Log:
Be more clever when allocating memory for log item to be escaped.
This should save about 70-100 bytes in the request pool with the default
config.
Modified:
httpd/httpd/trunk/server/util.c
Modified: httpd/httpd/trunk/server/util.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1461869r1=1461868r2=1461869view=diff
==
--- httpd/httpd/trunk/server/util.c (original)
+++ httpd/httpd/trunk/server/util.c Wed Mar 27 21:57:44 2013
@@ -1850,12 +1850,26 @@ AP_DECLARE(char *) ap_escape_logitem(apr
char *ret;
unsigned char *d;
const unsigned char *s;
+int length = 0;
if (!str) {
return NULL;
}
-ret = apr_palloc(p, 4 * strlen(str) + 1); /* Be safe */
+/* First, compute the space needed for the escaped string.
+ * This could be tweaked a bit for '\b', '\n'... These characters
+ * should not be common, so do not bother. */
+s = (const unsigned char *)str;
+for (; *s; ++s) {
+if (TEST_CHAR(*s, T_ESCAPE_LOGITEM)) {
+length += 4;/* for '\\' + c2x() */
+}
+else {
+length++;
+}
+}
+
+ret = apr_palloc(p, length + 1);
d = (unsigned char *)ret;
Here we treat CPU against memory. AFAIK this function is in the hot code
path, because it is used for many of the fields used in the default
access log format and also in many custom ones. All other uses seem to
be either trace log messages, some (few) error log messages and
mod_status output. For those cases I think neither the CPU nor the
Memory differences are important.
In the access log case it seems the typical fields which are run through
ap_escape_logitem() should rarely contain characters to escape. So an
alternative strategy would be to simply copy the string if we don't find
a character to escape. The second check for each char is not necessary then.
A quick draft patch is at
http://people.apache.org/~rjung/patches/ap_escape_logitem_enhanced.patch
It compiles, but I haven't really tested it. You get the idea though.
In the case of no escapes, it should be quite a bit faster. In the case
of escapes it shouldn't make a noticeable difference in any direction.
Regards,
Rainer
