Issue 78496

Created attachment 45939 [details]
patch

@mmeeks:  First of all (apart from the usual nitpicking that the STL hash of
course is not rather inefficient, as there is no such thing as the STL hash): 
*What* takes 56million pseudo-cycles less and thus becomes ~3% faster? --- That
is, is it worth it at all?

sb: no idea if stl::hash_set suggests a hash or not, a chestnut there ?
sb: the saving is on startup time - during which the intern hash is mostly
populated.
Did I mention it saves 170kb as well ? ;-)
Oh, and now I look, I didn't implement the simple prime table, so I'll do that &
re-measure the performance win.

Created attachment 45956 [details]
updated patch

@mmeeks:

- [std::hash_map, despite the name, is a kind of extension available in STLport,
but not Standard C++]

- If the patch is worthwhile (considering the tradeoff between increased code
complexity/refraining from reuse on the one hand and improved space/time
efficiency on the other hand---don't take me wrong, I'm just stating that here
neutrally):

 - commenting out #include "precompiled_sal.hxx" breaks the Windows build, I think

 - why leave the old stuff in #if 0?

> If the patch is worthwhile (considering the tradeoff between increased code

Sure - so, in raw %age terms; the hash is around 30% faster; and also somewhat
smaller. Clearly in more 'intern'-heavy code, that should result in >3% wins -
as we use intern more across the code-base, that should work nicely. The code
size increase is 188 lines instead of 84.

>commenting out #include "precompiled_sal.hxx" breaks the Windows build, I think

Good good ;-)

>why leave the old stuff in #if 0?

Makes the patch easy for you to read & on merge should be easy to cut the old
stuff out.

reassign for review.

The review already happened, I would say.  Micheal, feel free to integrate.

Stefan - is there any chance we can get this included into some other CWS, to
reduce test-tool, building thrash ?

Sorry, I have no CWS open or planned where we could put this into (done with OOo
2.4).  Either find somebody else who has, or create one yourself.

Created attachment 50131 [details]
fix previous patch ;-)

integrating in cbosdo06:
http://hg.services.openoffice.org/cws/cbosdo06/rev/dad2c5cc9342

Created attachment 69752 [details]
Patch adapted to DEV300

sb: could you verify it in cbosdo06?

@cedricbosdo: <http://hg.services.openoffice.org/cws/cbosdo06/rev/dad2c5cc9342>

- still contains #if 0 and "start here" cruft (see <#desc6>)

- still drops precompiled_sal.hxx (see <#desc6>)

- adds extern "C" at various places for no apparent reason (and this was not in
the originally attached sal-strintern-speed.diff); if functions shall be local
to the compilation unit, mark them "static"

sb: Those have been fixed in
<http://hg.services.openoffice.org/cws/cbosdo06/rev/b984532be182>

Have they?  Funny "start here" and dubious extern "C" are still there, and why
re-include apparently non-needed rtl/allocator.hxx?

Reopening issue.
According to Release Status Meeting this week (see
http://wiki.services.openoffice.org/wiki/ReleaseStatus_Minutes#2010-09-13)
setting target to OOo 3.3.

Correction: Set Target to OOo 3.4
sba -> cedricbosdo: As discussed today in IRC, back to you.

set target 3.x since not relevant for 3.4 release.

SBA->SB: as discussed with MBA, please proceed, thx.

I'm adding this comment to all open issues with Issue Type == PATCH.  We have 220 such issues, many of them quite old.  I apologize for that.  

We need your help in prioritizing which patches should be integrated into our next release, Apache OpenOffice 4.0.

If you have submitted a patch and think it is applicable for AOO 4.0, please respond with a comment to let us know.

On the other hand, if the patch is no longer relevant, please let us know that as well.

If you have any general questions or want to discuss this further, please send a note to our dev mailing list:  dev@openoffice.apache.org

Thanks!

-Rob

Reset assigne to the default "issues@openoffice.apache.org".