svn commit: r1598273 - in /subversion/trunk/subversion: libsvn_fs_fs/index.c libsvn_fs_x/index.c libsvn_ra_svn/marshal.c

[st...@apache.org]

Author: stefan2
Date: Thu May 29 12:27:42 2014
New Revision: 1598273

URL: http://svn.apache.org/r1598273
Log:
Follow-up to r1597962:
Update commentary on functions that use the SVN__FORCE_INLINE macro.
No functional change.

* subversion/libsvn_fs_fs/index.c
  (packed_stream_read): Explain our usage of SVN__PREVENT_INLINE in
                        more detail, using the correct function names.
  (packed_stream_get): Update commentary on SVN__FORCE_INLINE.

* subversion/libsvn_fs_x/index.c
  (packed_stream_read,
   packed_stream_get): As above for FSFS.

* subversion/libsvn_ra_svn/marshal.c
  (readbuf_getchar): Say why we use SVN__FORCE_INLINE here.

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/index.c
    subversion/trunk/subversion/libsvn_fs_x/index.c
    subversion/trunk/subversion/libsvn_ra_svn/marshal.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/index.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/index.c?rev=1598273&r1=1598272&r2=1598273&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/index.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/index.c Thu May 29 12:27:42 2014
@@ -204,8 +204,9 @@ stream_error_create(svn_fs_fs__packed_nu
 /* Read up to MAX_NUMBER_PREFETCH numbers from the STREAM->NEXT_OFFSET in
  * STREAM->FILE and buffer them.
  *
- * We don't want GCC and others to inline this function into get() because
- * it prevents get() from being inlined itself.
+ * We don't want GCC and others to inline this (infrequently called)
+ * function into packed_stream_get() because it prevents the latter from
+ * being inlined itself.
  */
 SVN__PREVENT_INLINE
 static svn_error_t *
@@ -343,8 +344,10 @@ svn_fs_fs__packed_stream_close(svn_fs_fs
 }
 
 /*
- * The forced inline is required as e.g. GCC would inline read() into here
- * instead of lining the simple buffer access into callers of get().
+ * The forced inline is required for performance reasons:  This is a very
+ * hot code path (called for every item we read) but e.g. GCC would rather
+ * chose to inline packed_stream_read() here, preventing packed_stream_get
+ * from being inlined itself.
  */
 SVN__FORCE_INLINE
 static svn_error_t*

Modified: subversion/trunk/subversion/libsvn_fs_x/index.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/index.c?rev=1598273&r1=1598272&r2=1598273&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/index.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/index.c Thu May 29 12:27:42 2014
@@ -209,8 +209,9 @@ stream_error_create(packed_number_stream
 /* Read up to MAX_NUMBER_PREFETCH numbers from the STREAM->NEXT_OFFSET in
  * STREAM->FILE and buffer them.
  *
- * We don't want GCC and others to inline this function into get() because
- * it prevents get() from being inlined itself.
+ * We don't want GCC and others to inline this (infrequently called)
+ * function into packed_stream_get() because it prevents the latter from
+ * being inlined itself.
  */
 SVN__PREVENT_INLINE
 static svn_error_t *
@@ -348,8 +349,10 @@ packed_stream_close(packed_number_stream
 }
 
 /*
- * The forced inline is required as e.g. GCC would inline read() into here
- * instead of lining the simple buffer access into callers of get().
+ * The forced inline is required for performance reasons:  This is a very
+ * hot code path (called for every item we read) but e.g. GCC would rather
+ * chose to inline packed_stream_read() here, preventing packed_stream_get
+ * from being inlined itself.
  */
 SVN__FORCE_INLINE
 static svn_error_t*

Modified: subversion/trunk/subversion/libsvn_ra_svn/marshal.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/marshal.c?rev=1598273&r1=1598272&r2=1598273&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_svn/marshal.c (original)
+++ subversion/trunk/subversion/libsvn_ra_svn/marshal.c Thu May 29 12:27:42 2014
@@ -394,6 +394,10 @@ static svn_error_t *readbuf_fill(svn_ra_
   return SVN_NO_ERROR;
 }
 
+/* This is a hot function calling a cold function.  GCC and others tend to
+ * inline the cold sub-function instead of this hot one.  Therefore, be
+ * very insistent on lining this one.  It is not a correctness issue, though.
+ */
 static SVN__FORCE_INLINE svn_error_t *
 readbuf_getchar(svn_ra_svn_conn_t *conn, apr_pool_t *pool, char *result)
 {