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 2012/05/14 21:12:14 UTC
svn commit: r1338350 - in /subversion/trunk/subversion: libsvn_subr/io.c
libsvn_subr/skel.c libsvn_wc/wc_db.c
Author: stsp
Date: Mon May 14 19:12:13 2012
New Revision: 1338350
URL: http://svn.apache.org/viewvc?rev=1338350&view=rev
Log:
Use svn_hash__make() instead of apr_hash_make() in select places to ensure
stable ordering of output. This commit makes the output of 'svn status'
and 'svn proplist' stable.
* subversion/libsvn_subr/skel.c
(svn_skel__parse_proplist): Return a hash table with stable ordering.
* subversion/libsvn_subr/io.c
(svn_io_get_dirents3): Return a hash table with stable ordering.
* subversion/libsvn_wc/wc_db.c
(gather_children, gather_children2): Use a hash table with stable ordering
for gathered children.
(svn_wc__db_read_children_info): Return hash tables with stable ordering
for nodes and conflicted nodes.
Modified:
subversion/trunk/subversion/libsvn_subr/io.c
subversion/trunk/subversion/libsvn_subr/skel.c
subversion/trunk/subversion/libsvn_wc/wc_db.c
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1338350&r1=1338349&r2=1338350&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Mon May 14 19:12:13 2012
@@ -65,6 +65,7 @@
#include "private/svn_atomic.h"
#include "private/svn_io_private.h"
+#include "private/svn_subr_private.h"
#define SVN_SLEEP_ENV_VAR "SVN_I_LOVE_CORRUPTED_WORKING_COPIES_SO_DISABLE_SLEEP_FOR_TIMESTAMPS"
@@ -2389,7 +2390,7 @@ svn_io_get_dirents3(apr_hash_t **dirents
if (!only_check_type)
flags |= APR_FINFO_SIZE | APR_FINFO_MTIME;
- *dirents = apr_hash_make(result_pool);
+ *dirents = svn_hash__make(result_pool);
SVN_ERR(svn_io_dir_open(&this_dir, path, scratch_pool));
Modified: subversion/trunk/subversion/libsvn_subr/skel.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/skel.c?rev=1338350&r1=1338349&r2=1338350&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/skel.c (original)
+++ subversion/trunk/subversion/libsvn_subr/skel.c Mon May 14 19:12:13 2012
@@ -25,6 +25,7 @@
#include "svn_error.h"
#include "private/svn_skel.h"
#include "private/svn_string_private.h"
+#include "private/svn_subr_private.h"
/* Parsing skeletons. */
@@ -680,7 +681,7 @@ svn_skel__parse_proplist(apr_hash_t **pr
return skel_err("proplist");
/* Create the returned structure */
- proplist = apr_hash_make(pool);
+ proplist = svn_hash__make(pool);
for (elt = skel->children; elt; elt = elt->next->next)
{
svn_string_t *value = svn_string_ncreate(elt->next->data,
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1338350&r1=1338349&r2=1338350&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon May 14 19:12:13 2012
@@ -51,6 +51,7 @@
#include "private/svn_skel.h"
#include "private/svn_wc_private.h"
#include "private/svn_token.h"
+#include "private/svn_subr_private.h"
#define NOT_IMPLEMENTED() SVN__NOT_IMPLEMENTED()
@@ -1186,7 +1187,7 @@ gather_children2(const apr_array_header_
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
{
- apr_hash_t *names_hash = apr_hash_make(scratch_pool);
+ apr_hash_t *names_hash = svn_hash__make(scratch_pool);
apr_array_header_t *names_array;
/* All of the names get allocated in RESULT_POOL. It
@@ -1210,7 +1211,7 @@ gather_children(const apr_array_header_t
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
{
- apr_hash_t *names_hash = apr_hash_make(scratch_pool);
+ apr_hash_t *names_hash = svn_hash__make(scratch_pool);
apr_array_header_t *names_array;
/* All of the names get allocated in RESULT_POOL. It
@@ -2210,7 +2211,7 @@ svn_wc__db_base_get_children_info(apr_ha
dir_abspath, scratch_pool, scratch_pool));
VERIFY_USABLE_WCROOT(wcroot);
- *nodes = apr_hash_make(result_pool);
+ *nodes = svn_hash__make(result_pool);
SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
STMT_SELECT_BASE_CHILDREN_INFO));
@@ -7687,8 +7688,8 @@ svn_wc__db_read_children_info(apr_hash_t
svn_wc__db_wcroot_t *wcroot;
const char *dir_relpath;
- *conflicts = apr_hash_make(result_pool);
- *nodes = apr_hash_make(result_pool);
+ *conflicts = svn_hash__make(result_pool);
+ *nodes = svn_hash__make(result_pool);
SVN_ERR_ASSERT(svn_dirent_is_absolute(dir_abspath));
SVN_ERR(svn_wc__db_wcroot_parse_local_abspath(&wcroot, &dir_relpath, db,
@@ -7881,7 +7882,7 @@ svn_wc__db_read_children_walker_info(apr
SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, dir_relpath));
SVN_ERR(svn_sqlite__step(&have_row, stmt));
- *nodes = apr_hash_make(result_pool);
+ *nodes = svn_hash__make(result_pool);
while (have_row)
{
struct svn_wc__db_walker_info_t *child;
Re: svn commit: r1338350 - in /subversion/trunk/subversion:
libsvn_subr/io.c libsvn_subr/skel.c libsvn_wc/wc_db.c
Posted by Stefan Sperling <st...@elego.de>.
On Tue, May 15, 2012 at 01:02:40PM +0200, Branko Čibej wrote:
> That doesn't really make much sense. Only the test suite is intersted in
> /stable/ key ordering, and that's just because the test result
> comparison functions require it.
I don't think they do at present because the test suite would be
failing left and right if they did. Looking at the code, we build
a tree from status output and then compare this tree to an expected
tree. It looks as if the order of paths in status output is irrelevant.
> Users will not care about stable output unless it's sorted.
Sure. All I'm saying is that if the stable ordering happens to be
sorted, we're saving a qsort(). I don't care about an extra qsort()
myself. But Bert apparently does, and I'm trying to reach consensus.
> On the subject of hash functions, I doubt you can go much faster than
> what APR already has, except for saving the few bytes of the randomizer
> added to the key.
I didn't actually mean to make the claim that it was faster. I just
blindly believe the docstring of svn_hash__make() which claims this.
Re: svn commit: r1338350 - in /subversion/trunk/subversion: libsvn_subr/io.c
libsvn_subr/skel.c libsvn_wc/wc_db.c
Posted by Branko Čibej <br...@apache.org>.
On 15.05.2012 12:56, Stefan Sperling wrote:
> On Tue, May 15, 2012 at 05:38:41AM -0400, Greg Stein wrote:
>> If Bert can somehow demonstrate how a qsort() is troublesome, then ...
>> sure. And if he can somehow show that the cpu clocks are more
>> important than the long-term ability of our community to maintain svn
>> against the coupling of "skel prop parsing" and "svn status output"...
>> then, wow. That would be an amazing demonstration.
> I'd also like to note that svn_sort__hash() does not invoke qsort()
> if hash keys happen to be returned in sorted order. It only performs
> a linear scan in that case.
>
> So... a compromise could be to keep using svn_hash__make(), with a comment
> that explains that we'd like to our own faster and stable hash function,
> in particular because stable key ordering is more desirable for our use case
> than random key ordering.
That doesn't really make much sense. Only the test suite is intersted in
/stable/ key ordering, and that's just because the test result
comparison functions require it. Users will not care about stable output
unless it's sorted.
On the subject of hash functions, I doubt you can go much faster than
what APR already has, except for saving the few bytes of the randomizer
added to the key.
-- Brane
Re: svn commit: r1338350 - in /subversion/trunk/subversion:
libsvn_subr/io.c libsvn_subr/skel.c libsvn_wc/wc_db.c
Posted by Stefan Sperling <st...@elego.de>.
On Tue, May 15, 2012 at 05:38:41AM -0400, Greg Stein wrote:
> If Bert can somehow demonstrate how a qsort() is troublesome, then ...
> sure. And if he can somehow show that the cpu clocks are more
> important than the long-term ability of our community to maintain svn
> against the coupling of "skel prop parsing" and "svn status output"...
> then, wow. That would be an amazing demonstration.
I'd also like to note that svn_sort__hash() does not invoke qsort()
if hash keys happen to be returned in sorted order. It only performs
a linear scan in that case.
So... a compromise could be to keep using svn_hash__make(), with a comment
that explains that we'd like to our own faster and stable hash function,
in particular because stable key ordering is more desirable for our use case
than random key ordering. But we also run svn_sort__hash() on the list of
dirents to guarantee sorted output. This will usually not invoke qsort(),
just scan the list.
That way, we have a clear separation of concerns, and guaranteed sorted
output, and no unnecessary qsort() calls. And hopefully everyone is happy.
Re: svn commit: r1338350 - in /subversion/trunk/subversion:
libsvn_subr/io.c libsvn_subr/skel.c libsvn_wc/wc_db.c
Posted by Greg Stein <gs...@gmail.com>.
On Tue, May 15, 2012 at 5:04 AM, Stefan Sperling <st...@elego.de> wrote:
> On Mon, May 14, 2012 at 11:43:40PM -0400, Greg Stein wrote:
>...
>> My concern is keeping the lower levels free to do what is best for
>> them. The change in this commit is *very* tenuous to its intended
>> purpose. That is going to break at some point. Basically cause it
>> introduces some awful coupling between our subsystems.
>
> Well, what it really does it revert to pre-APR-1.4.6 behaviour.
> I.e. it changes the behaviour back to what it was independent
> of the APR version used.
>
> The intent is to restore a side-effect of this behaviour (stable output).
> We want to keep this side-effect, so we can either keep relying on our
> implicit assumption that a stable hash will produce ordered output,
> or we can sort. I don't care either way. I just want sorted/stable output.
Sorted output is nicer for users.
("stable" just helps to de-confuse)
>> IOW, I'll take a couple milliseconds to perform a sort over the
>> long-term maintenance burden of coupled systems.
>
> You'll have to argue this one with Bert, then :P
If Bert can somehow demonstrate how a qsort() is troublesome, then ...
sure. And if he can somehow show that the cpu clocks are more
important than the long-term ability of our community to maintain svn
against the coupling of "skel prop parsing" and "svn status output"...
then, wow. That would be an amazing demonstration.
> I already told him on IRC I believe sorting dirents in memory won't
> make any noticable difference since we're not going to disk and we're
> not accessing sqlite while sorting, and those expensive operations
> are what 'status' does all the time. But he insisted.
Decisions, discussions, and arguments on list, please.
(all is good; that's what we're doing now)
>> [ to beat this dead horse some more: not a single one of your changes
>> mentioned WHY svn_hash__make() was used instead of apr_hash_make();
>> somebody coming along will have absolutely no idea why that was done;
>> so they might switch it back because there is zero apparent effect at
>> doing so; at a minimum, a comment saying "we sort our skel proplists
>> so that 'svn status' (yah, that thing 15 layers up) can have stable
>> output." ]
>
> The docstring of svn_hash__make() makes the difference quite clear.
That docstring is clear for *that function*. Your change made
absolutely no in-code clarification for *WHY* it was necessary to use
that function. "Go look at the func's docstring" *still* does not make
it clear on *WHY* that hash creator was chosen.
"The grapes are purple" ... yah, great. Facts are stated. Why did you
choose those over the green grapes?
Write a comment for that call to svn_hash__make(). Write a good one.
One that won't make people twitch. One that won't make you squirm
because you're stretching the coupling between skels and the svn
cmdline. I'll buy a round if you can muster that creativity :-D
>...
> Here's my original patch (which won't apply cleanly to HEAD anymore, BTW).
> Note that this still sorts stuff for the benefit of 'svn status' "15
> layers up". I can't find a better place to perform sorting.
Right. As you noted: once you hit a single-node callback, then the
receiver has no choice to sort. But the status walker is much, much
closer to the output than deserialization of some characters.
>...
Cheers,
-g
Re: svn commit: r1338350 - in /subversion/trunk/subversion:
libsvn_subr/io.c libsvn_subr/skel.c libsvn_wc/wc_db.c
Posted by Stefan Sperling <st...@elego.de>.
On Mon, May 14, 2012 at 11:43:40PM -0400, Greg Stein wrote:
> On Mon, May 14, 2012 at 7:05 PM, Stefan Sperling <st...@elego.de> wrote:
> > On Mon, May 14, 2012 at 05:42:01PM -0400, Greg Stein wrote:
> > Yes, it is more of UI level issue that does not belong into the
> > core libraries. It should be done by the UI.
> >
> > But the UI gets one path per callback invocation.
> > So it cannot control the order itself without buffering everything.
> > That is not acceptable either.
>
> Right. What are the callback drivers? I'm guessing the status walker.
> Throwing an O(n log n) key sort into that will not cause any
> appreciable overheads, relative to the I/O that is driving the overall
> operation.
>
> There is a *user* benefit for sorted keys, rather than just stable keys.
Yes. Sorting is the only way to guarantee consistent output.
> > I had a patch that used a sorted array of hash table keys (still in
> > libsvn_wc, not in 'svn') but Bert objected to that due to performance
> > concerns and suggested to use the newly added stable hash table instead.
>
> I'd like to see that patch. I really can't imagine that we're talking
> any systemic performance here.
Glad I kept a copy. See below.
> My concern is keeping the lower levels free to do what is best for
> them. The change in this commit is *very* tenuous to its intended
> purpose. That is going to break at some point. Basically cause it
> introduces some awful coupling between our subsystems.
Well, what it really does it revert to pre-APR-1.4.6 behaviour.
I.e. it changes the behaviour back to what it was independent
of the APR version used.
The intent is to restore a side-effect of this behaviour (stable output).
We want to keep this side-effect, so we can either keep relying on our
implicit assumption that a stable hash will produce ordered output,
or we can sort. I don't care either way. I just want sorted/stable output.
> IOW, I'll take a couple milliseconds to perform a sort over the
> long-term maintenance burden of coupled systems.
You'll have to argue this one with Bert, then :P
I already told him on IRC I believe sorting dirents in memory won't
make any noticable difference since we're not going to disk and we're
not accessing sqlite while sorting, and those expensive operations
are what 'status' does all the time. But he insisted.
> [ to beat this dead horse some more: not a single one of your changes
> mentioned WHY svn_hash__make() was used instead of apr_hash_make();
> somebody coming along will have absolutely no idea why that was done;
> so they might switch it back because there is zero apparent effect at
> doing so; at a minimum, a comment saying "we sort our skel proplists
> so that 'svn status' (yah, that thing 15 layers up) can have stable
> output." ]
The docstring of svn_hash__make() makes the difference quite clear.
> > The unordered output sucks. This commit was the result of a user
> > complaining about unordered output of 'svn status' after he upgraded
> > to ubuntu 12.04 which includes apr-1.4.6. Unordered output which used
> > to be ordered is perceived as a regression by our users. IMO we can't
> > just ignore this problem.
>
> Sure.
>
> > What would you suggest to do instead? I'm open to suggestions.
>
> Move the code further up. Adding a qsort() somewhere is not going to
> be an issue except for some of the most pathological directory sizes.
> And I will bet directories of that size have other problems relative
> to that qsort().
Here's my original patch (which won't apply cleanly to HEAD anymore, BTW).
Note that this still sorts stuff for the benefit of 'svn status' "15
layers up". I can't find a better place to perform sorting.
And I suppose one could argue that the committed change was less
intrusive while still having the desired side-effect :)
Index: subversion/libsvn_wc/status.c
===================================================================
--- subversion/libsvn_wc/status.c (revision 1338291)
+++ subversion/libsvn_wc/status.c (working copy)
@@ -41,6 +41,7 @@
#include "svn_config.h"
#include "svn_time.h"
#include "svn_hash.h"
+#include "svn_sorts.h"
#include "svn_private_config.h"
@@ -1298,15 +1299,16 @@ get_dir_status(const struct walk_status_baton *wb,
void *cancel_baton,
apr_pool_t *scratch_pool)
{
- apr_hash_index_t *hi;
const char *dir_repos_root_url;
const char *dir_repos_relpath;
const char *dir_repos_uuid;
svn_boolean_t dir_has_props;
apr_hash_t *dirents, *nodes, *conflicts, *all_children;
+ apr_array_header_t *sorted_children;
apr_array_header_t *collected_ignore_patterns = NULL;
apr_pool_t *iterpool, *subpool = svn_pool_create(scratch_pool);
svn_error_t *err;
+ int i;
if (cancel_func)
SVN_ERR(cancel_func(cancel_baton));
@@ -1390,17 +1392,23 @@ get_dir_status(const struct walk_status_baton *wb,
dir_has_props = (dir_info->had_props || dir_info->props_mod);
/* Walk all the children of this directory. */
- for (hi = apr_hash_first(subpool, all_children); hi; hi = apr_hash_next(hi))
+ sorted_children = svn_sort__hash(all_children,
+ svn_sort_compare_items_as_paths,
+ subpool);
+ for (i = 0; i < sorted_children->nelts; i++)
{
const void *key;
apr_ssize_t klen;
+ svn_sort__item_t item;
const char *child_abspath;
svn_io_dirent2_t *child_dirent;
const struct svn_wc__db_info_t *child_info;
svn_pool_clear(iterpool);
- apr_hash_this(hi, &key, &klen, NULL);
+ item = APR_ARRAY_IDX(sorted_children, i, svn_sort__item_t);
+ key = item.key;
+ klen = item.klen;
child_abspath = svn_dirent_join(local_abspath, key, iterpool);
Re: svn commit: r1338350 - in /subversion/trunk/subversion:
libsvn_subr/io.c libsvn_subr/skel.c libsvn_wc/wc_db.c
Posted by Greg Stein <gs...@gmail.com>.
On Mon, May 14, 2012 at 7:05 PM, Stefan Sperling <st...@elego.de> wrote:
> On Mon, May 14, 2012 at 05:42:01PM -0400, Greg Stein wrote:
>> On Mon, May 14, 2012 at 3:12 PM, <st...@apache.org> wrote:
>> > Author: stsp
>> > Date: Mon May 14 19:12:13 2012
>> > New Revision: 1338350
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1338350&view=rev
>> > Log:
>> > Use svn_hash__make() instead of apr_hash_make() in select places to ensure
>> > stable ordering of output. This commit makes the output of 'svn status'
>> > and 'svn proplist' stable.
>>
>> I think you're solving this at the wrong level. Why should the
>> functions below care about stable ordering? These are hash tables,
>> which *by definition* are "unordered". With these changes, you're
>> affecting *all* users rather than just the places that are important.
>>
>> If you want stable ordering, then at some higher level _where ordering
>> matters_, get an array of sorted keys and walk the hash table that
>> way.
>>
>> I'm somewhere between -0.5 and -1 on this commit. I feel pretty
>> strongly that this is not a correct solution.
>>
>> Thanks,
>> -g
>
> Yes, it is more of UI level issue that does not belong into the
> core libraries. It should be done by the UI.
>
> But the UI gets one path per callback invocation.
> So it cannot control the order itself without buffering everything.
> That is not acceptable either.
Right. What are the callback drivers? I'm guessing the status walker.
Throwing an O(n log n) key sort into that will not cause any
appreciable overheads, relative to the I/O that is driving the overall
operation.
There is a *user* benefit for sorted keys, rather than just stable keys.
(ra_serf will always have some variability due to network performance,
but we can fix 'svn st' output at a minimum)
> I had a patch that used a sorted array of hash table keys (still in
> libsvn_wc, not in 'svn') but Bert objected to that due to performance
> concerns and suggested to use the newly added stable hash table instead.
I'd like to see that patch. I really can't imagine that we're talking
any systemic performance here.
My concern is keeping the lower levels free to do what is best for
them. The change in this commit is *very* tenuous to its intended
purpose. That is going to break at some point. Basically cause it
introduces some awful coupling between our subsystems.
IOW, I'll take a couple milliseconds to perform a sort over the
long-term maintenance burden of coupled systems.
[ to beat this dead horse some more: not a single one of your changes
mentioned WHY svn_hash__make() was used instead of apr_hash_make();
somebody coming along will have absolutely no idea why that was done;
so they might switch it back because there is zero apparent effect at
doing so; at a minimum, a comment saying "we sort our skel proplists
so that 'svn status' (yah, that thing 15 layers up) can have stable
output." ]
> The unordered output sucks. This commit was the result of a user
> complaining about unordered output of 'svn status' after he upgraded
> to ubuntu 12.04 which includes apr-1.4.6. Unordered output which used
> to be ordered is perceived as a regression by our users. IMO we can't
> just ignore this problem.
Sure.
> What would you suggest to do instead? I'm open to suggestions.
Move the code further up. Adding a qsort() somewhere is not going to
be an issue except for some of the most pathological directory sizes.
And I will bet directories of that size have other problems relative
to that qsort().
Cheers,
-g
Re: svn commit: r1338350 - in /subversion/trunk/subversion:
libsvn_subr/io.c libsvn_subr/skel.c libsvn_wc/wc_db.c
Posted by Stefan Sperling <st...@elego.de>.
On Mon, May 14, 2012 at 05:42:01PM -0400, Greg Stein wrote:
> On Mon, May 14, 2012 at 3:12 PM, <st...@apache.org> wrote:
> > Author: stsp
> > Date: Mon May 14 19:12:13 2012
> > New Revision: 1338350
> >
> > URL: http://svn.apache.org/viewvc?rev=1338350&view=rev
> > Log:
> > Use svn_hash__make() instead of apr_hash_make() in select places to ensure
> > stable ordering of output. This commit makes the output of 'svn status'
> > and 'svn proplist' stable.
>
> I think you're solving this at the wrong level. Why should the
> functions below care about stable ordering? These are hash tables,
> which *by definition* are "unordered". With these changes, you're
> affecting *all* users rather than just the places that are important.
>
> If you want stable ordering, then at some higher level _where ordering
> matters_, get an array of sorted keys and walk the hash table that
> way.
>
> I'm somewhere between -0.5 and -1 on this commit. I feel pretty
> strongly that this is not a correct solution.
>
> Thanks,
> -g
Yes, it is more of UI level issue that does not belong into the
core libraries. It should be done by the UI.
But the UI gets one path per callback invocation.
So it cannot control the order itself without buffering everything.
That is not acceptable either.
I had a patch that used a sorted array of hash table keys (still in
libsvn_wc, not in 'svn') but Bert objected to that due to performance
concerns and suggested to use the newly added stable hash table instead.
The unordered output sucks. This commit was the result of a user
complaining about unordered output of 'svn status' after he upgraded
to ubuntu 12.04 which includes apr-1.4.6. Unordered output which used
to be ordered is perceived as a regression by our users. IMO we can't
just ignore this problem.
What would you suggest to do instead? I'm open to suggestions.
Re: svn commit: r1338350 - in /subversion/trunk/subversion:
libsvn_subr/io.c libsvn_subr/skel.c libsvn_wc/wc_db.c
Posted by Greg Stein <gs...@gmail.com>.
On Mon, May 14, 2012 at 3:12 PM, <st...@apache.org> wrote:
> Author: stsp
> Date: Mon May 14 19:12:13 2012
> New Revision: 1338350
>
> URL: http://svn.apache.org/viewvc?rev=1338350&view=rev
> Log:
> Use svn_hash__make() instead of apr_hash_make() in select places to ensure
> stable ordering of output. This commit makes the output of 'svn status'
> and 'svn proplist' stable.
I think you're solving this at the wrong level. Why should the
functions below care about stable ordering? These are hash tables,
which *by definition* are "unordered". With these changes, you're
affecting *all* users rather than just the places that are important.
If you want stable ordering, then at some higher level _where ordering
matters_, get an array of sorted keys and walk the hash table that
way.
I'm somewhere between -0.5 and -1 on this commit. I feel pretty
strongly that this is not a correct solution.
Thanks,
-g