This is downright horrible for code maintainability. We have a type for
revision numbers.
It's bad enough we don't have a stable type for revision numbers across
platforms. But now you're abusing it and using 32-bit integers in some places
for them and 64-bit values in others.
Now that you're doing this I'm now of the opinion that I should veto the code
that limits them to 32-bits at the RA layer. Because all it's done is
encourage us to do things like this.
On 3/7/14, 2:11 PM, stefan2_at_apache.org wrote:
> Author: stefan2
> Date: Fri Mar 7 22:11:24 2014
> New Revision: 1575428
>
> URL: http://svn.apache.org/r1575428
> Log:
> In FSFS, remove padding from cache key structs to simplify handling.
> Also, document why we want them to be 16 bytes whenever we can help it.
>
> * subversion/libsvn_fs_fs/fs.h
> (pair_cache_key_t): Widen revision type to eliminate padding on
> platforms with sizeof(long) != 8. Document the
> struct size rationale.
> (window_cache_key_t): Document the struct size rationale.
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_fs/fs.h
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/fs.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs.h?rev=1575428&r1=1575427&r2=1575428&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/fs.h (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs.h Fri Mar 7 22:11:24 2014
> @@ -254,20 +254,27 @@ typedef struct fs_fs_dag_cache_t fs_fs_d
>
> /* Key type for all caches that use revision + offset / counter as key.
>
> - NOTE: always initialize this using calloc() or '= {0};'! This is used
> - as a cache key and the padding bytes on 32 bit archs should be zero for
> - cache effectiveness. */
> + Note: Cache keys should be 16 bytes for best performance and there
> + should be no padding. */
> typedef struct pair_cache_key_t
> {
> - svn_revnum_t revision;
> + /* The object's revision. Use the 64 data type to prevent padding. */
> + apr_int64_t revision;
>
> + /* Sub-address: item index, revprop generation, packed flag, etc. */
> apr_int64_t second;
> } pair_cache_key_t;
>
> -/* Key type that identifies a txdelta window. */
> +/* Key type that identifies a txdelta window.
> +
> + Note: Cache keys should be 16 bytes for best performance and there
> + should be no padding. */
> typedef struct window_cache_key_t
> {
> - /* Revision that contains the representation */
> + /* Revision that contains the representation.
> + We limit this to 32 bit because it revision numbers are practically
> + limited to 32 bits by the RA layer, ATM, and we want to keep this
> + struct 16 bytes long. */
> apr_uint32_t revision;
>
> /* Window number within that representation */
>
>
Received on 2014-03-07 23:33:00 CET