You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2010/10/27 22:47:06 UTC
svn commit: r1028094 -
/subversion/branches/performance/subversion/libsvn_subr/svn_string.c
Author: stefan2
Date: Wed Oct 27 20:47:06 2010
New Revision: 1028094
URL: http://svn.apache.org/viewvc?rev=1028094&view=rev
Log:
If we allocate stringbufs, their actual capacity allocated by APR
is often larger than requested. Take advantage of that by requesting
an enlarged buffer in the first place. Especially for short or empty
strings, this saves some re-allocations once they grow.
* subversion/libsvn_subr/svn_string.c
(svn_stringbuf_ncreate): allocate a buffer of aligned size
Modified:
subversion/branches/performance/subversion/libsvn_subr/svn_string.c
Modified: subversion/branches/performance/subversion/libsvn_subr/svn_string.c
URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/svn_string.c?rev=1028094&r1=1028093&r2=1028094&view=diff
==============================================================================
--- subversion/branches/performance/subversion/libsvn_subr/svn_string.c (original)
+++ subversion/branches/performance/subversion/libsvn_subr/svn_string.c Wed Oct 27 20:47:06 2010
@@ -266,9 +266,21 @@ svn_stringbuf_create_ensure(apr_size_t b
svn_stringbuf_t *
svn_stringbuf_ncreate(const char *bytes, apr_size_t size, apr_pool_t *pool)
{
- /* Ensure string buffer of size + 1 */
- svn_stringbuf_t *strbuf = svn_stringbuf_create_ensure(size, pool);
+ char *data;
+ /* apr_palloc will allocate multiples of 8.
+ * Thus, we would waste some of that memory if we stuck to the
+ * smaller size. Note that this is safe even if apr_palloc would
+ * use some other aligment or none at all. */
+ apr_size_t aligned_size = APR_ALIGN_DEFAULT(size + 1) - 1;
+
+ /* Ensure string buffer of aligned_size + 1.
+ * This should allocate the same amount of memory as "size" would. */
+ svn_stringbuf_t *strbuf = svn_stringbuf_create_ensure(aligned_size, pool);
+
+ /* Actually, aligned_size+1 would be faster but we cannot be entirely
+ * sure that the source string has been aligned properly such that
+ * all the extra bytes would actually come from addressible memory.*/
memcpy(strbuf->data, bytes, size);
/* Null termination is the convention -- even if we suspect the data
Re: svn commit: r1028094 - /subversion/branches/performance/subversion/libsvn_subr/svn_string.c
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Wed, Oct 27, 2010 at 3:47 PM, <st...@apache.org> wrote:
> Author: stefan2
> Date: Wed Oct 27 20:47:06 2010
> New Revision: 1028094
>
> URL: http://svn.apache.org/viewvc?rev=1028094&view=rev
> Log:
> If we allocate stringbufs, their actual capacity allocated by APR
> is often larger than requested. Take advantage of that by requesting
> an enlarged buffer in the first place. Especially for short or empty
> strings, this saves some re-allocations once they grow.
>
> * subversion/libsvn_subr/svn_string.c
> (svn_stringbuf_ncreate): allocate a buffer of aligned size
>
> Modified:
> subversion/branches/performance/subversion/libsvn_subr/svn_string.c
>
> Modified: subversion/branches/performance/subversion/libsvn_subr/svn_string.c
> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/svn_string.c?rev=1028094&r1=1028093&r2=1028094&view=diff
> ==============================================================================
> --- subversion/branches/performance/subversion/libsvn_subr/svn_string.c (original)
> +++ subversion/branches/performance/subversion/libsvn_subr/svn_string.c Wed Oct 27 20:47:06 2010
> @@ -266,9 +266,21 @@ svn_stringbuf_create_ensure(apr_size_t b
> svn_stringbuf_t *
> svn_stringbuf_ncreate(const char *bytes, apr_size_t size, apr_pool_t *pool)
> {
> - /* Ensure string buffer of size + 1 */
> - svn_stringbuf_t *strbuf = svn_stringbuf_create_ensure(size, pool);
> + char *data;
>
> + /* apr_palloc will allocate multiples of 8.
> + * Thus, we would waste some of that memory if we stuck to the
> + * smaller size. Note that this is safe even if apr_palloc would
> + * use some other aligment or none at all. */
> + apr_size_t aligned_size = APR_ALIGN_DEFAULT(size + 1) - 1;
Should this be const, or maybe even a macro at the top of the file?
(Thanks for the good docstring, though.)
> +
> + /* Ensure string buffer of aligned_size + 1.
> + * This should allocate the same amount of memory as "size" would. */
> + svn_stringbuf_t *strbuf = svn_stringbuf_create_ensure(aligned_size, pool);
> +
> + /* Actually, aligned_size+1 would be faster but we cannot be entirely
> + * sure that the source string has been aligned properly such that
> + * all the extra bytes would actually come from addressible memory.*/
> memcpy(strbuf->data, bytes, size);
>
> /* Null termination is the convention -- even if we suspect the data
>
>
>
Re: svn commit: r1028094 - /subversion/branches/performance/subversion/libsvn_subr/svn_string.c
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Wed, Oct 27, 2010 at 3:47 PM, <st...@apache.org> wrote:
> Author: stefan2
> Date: Wed Oct 27 20:47:06 2010
> New Revision: 1028094
>
> URL: http://svn.apache.org/viewvc?rev=1028094&view=rev
> Log:
> If we allocate stringbufs, their actual capacity allocated by APR
> is often larger than requested. Take advantage of that by requesting
> an enlarged buffer in the first place. Especially for short or empty
> strings, this saves some re-allocations once they grow.
>
> * subversion/libsvn_subr/svn_string.c
> (svn_stringbuf_ncreate): allocate a buffer of aligned size
>
> Modified:
> subversion/branches/performance/subversion/libsvn_subr/svn_string.c
>
> Modified: subversion/branches/performance/subversion/libsvn_subr/svn_string.c
> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/svn_string.c?rev=1028094&r1=1028093&r2=1028094&view=diff
> ==============================================================================
> --- subversion/branches/performance/subversion/libsvn_subr/svn_string.c (original)
> +++ subversion/branches/performance/subversion/libsvn_subr/svn_string.c Wed Oct 27 20:47:06 2010
> @@ -266,9 +266,21 @@ svn_stringbuf_create_ensure(apr_size_t b
> svn_stringbuf_t *
> svn_stringbuf_ncreate(const char *bytes, apr_size_t size, apr_pool_t *pool)
> {
> - /* Ensure string buffer of size + 1 */
> - svn_stringbuf_t *strbuf = svn_stringbuf_create_ensure(size, pool);
> + char *data;
>
> + /* apr_palloc will allocate multiples of 8.
> + * Thus, we would waste some of that memory if we stuck to the
> + * smaller size. Note that this is safe even if apr_palloc would
> + * use some other aligment or none at all. */
> + apr_size_t aligned_size = APR_ALIGN_DEFAULT(size + 1) - 1;
Should this be const, or maybe even a macro at the top of the file?
(Thanks for the good docstring, though.)
> +
> + /* Ensure string buffer of aligned_size + 1.
> + * This should allocate the same amount of memory as "size" would. */
> + svn_stringbuf_t *strbuf = svn_stringbuf_create_ensure(aligned_size, pool);
> +
> + /* Actually, aligned_size+1 would be faster but we cannot be entirely
> + * sure that the source string has been aligned properly such that
> + * all the extra bytes would actually come from addressible memory.*/
> memcpy(strbuf->data, bytes, size);
>
> /* Null termination is the convention -- even if we suspect the data
>
>
>
Re: svn commit: r1028094 -
/subversion/branches/performance/subversion/libsvn_subr/svn_string.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stefan2@apache.org wrote on Wed, Oct 27, 2010 at 20:47:06 -0000:
> Author: stefan2
> Date: Wed Oct 27 20:47:06 2010
> New Revision: 1028094
>
> URL: http://svn.apache.org/viewvc?rev=1028094&view=rev
> Log:
> If we allocate stringbufs, their actual capacity allocated by APR
> is often larger than requested. Take advantage of that by requesting
> an enlarged buffer in the first place. Especially for short or empty
> strings, this saves some re-allocations once they grow.
>
> * subversion/libsvn_subr/svn_string.c
> (svn_stringbuf_ncreate): allocate a buffer of aligned size
>
> Modified:
> subversion/branches/performance/subversion/libsvn_subr/svn_string.c
>
> Modified: subversion/branches/performance/subversion/libsvn_subr/svn_string.c
> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/svn_string.c?rev=1028094&r1=1028093&r2=1028094&view=diff
> ==============================================================================
> --- subversion/branches/performance/subversion/libsvn_subr/svn_string.c (original)
> +++ subversion/branches/performance/subversion/libsvn_subr/svn_string.c Wed Oct 27 20:47:06 2010
> @@ -266,9 +266,21 @@ svn_stringbuf_create_ensure(apr_size_t b
> svn_stringbuf_t *
> svn_stringbuf_ncreate(const char *bytes, apr_size_t size, apr_pool_t *pool)
> {
> - /* Ensure string buffer of size + 1 */
> - svn_stringbuf_t *strbuf = svn_stringbuf_create_ensure(size, pool);
> + char *data;
>
Is this new variable used anywhere?
> + /* apr_palloc will allocate multiples of 8.
> + * Thus, we would waste some of that memory if we stuck to the
> + * smaller size. Note that this is safe even if apr_palloc would
> + * use some other aligment or none at all. */
> + apr_size_t aligned_size = APR_ALIGN_DEFAULT(size + 1) - 1;
> +
So, this rounds the requested number of bytes (size+1) to the next
multiple of 8, then subtracts 1 from the result? Net effect being that
if we need 1 byte, we get 7; and if we need 2 or 7 bytes, then we get
7 bytes, right?
But if we need size+1=8 bytes, then.. I think we get aligned_size=7, and
svn_stringbuf_create_ensure() compensates because it allocates one byte
more than requested, right?
> + /* Ensure string buffer of aligned_size + 1.
> + * This should allocate the same amount of memory as "size" would. */
> + svn_stringbuf_t *strbuf = svn_stringbuf_create_ensure(aligned_size, pool);
> +
Could this "use aligned_size instead of size" logic be pushed down to
svn_stringbuf_create_ensure()?
> + /* Actually, aligned_size+1 would be faster but we cannot be entirely
> + * sure that the source string has been aligned properly such that
> + * all the extra bytes would actually come from addressible memory.*/
> memcpy(strbuf->data, bytes, size);
>
> /* Null termination is the convention -- even if we suspect the data
>
>