Hi Gatekeepers,
Please see the review comment from Matt Austern as following.
Although I am not a gatekeeper, I would make my suggestion any way:
1. to accept the change with wrapper around changes for now
2. prepare to submit this changes to GNU community, with additional
analysis as Matt suggested
3. submitter to own an action item to remove the ifdef when GNU library
accept this patch, officially make the incompatible ABI roll and when open64
match up the library in the future release
Any thoughts by gatekeeper community?
Regards,
Shin
---------- Forwarded message ----------
From: Matt Austern <aust...@google.com>
Date: Fri, Jun 3, 2011 at 2:37 PM
Subject: Re: [Open64-devel] Code Review Request for changes in STL tree
implementation to speed up set<>/map<>
To: Shin-Ming Liu <shinm...@gmail.com>
Cc: Sun Chan <sun.c...@gmail.com>
I have three general comments.
First, of course: if you're submitting this as a patch to libstdc++ then
you'll probably want to remove all of that __OPEN64_FAST_SET stuff.
Second: yes, I think it will be a problem that this change would break
binary compatibility. Not an insurmountable change. The libstdc++ people do
have a policy on ABI-breaking changes. (Basically, they're saving up a lot
of those changes and they'll release them all at once when they're ready to
release an incompatible libstdc++. I don't know exactly when that release
will happen, and possibly nobody knows, but it is planned.
Third: I'm not sure if this change is a good idea. I'm sure your
measurements (and those of the IBM people) are correct, and that this is a
speedup, but the cost is high: two extra pointers per node. If you've got
something like set<char*> then we'll currently use 5 words per node, so this
change will be a 40% space increase. SPEC typically deals with small test
cases where speed matters more than memory use, but I'm not sure it's as
good a tradeoff in general. It's not hard to imagine that there might be
other cases where it would be a slowdown because increasing node size could
decrease locality of reference.
If you do send this to the libstdc++ list, I'd definitely bring up the
time/space tradeoff explicitly so the people on that list can think about
it.
--Matt
------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Discover what all the cheering's about.
Get your free trial download today.
http://p.sf.net/sfu/quest-dev2dev2
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
