Opened 12 years ago

Closed 11 years ago

#1595 closed enhancement (fixed)

Refcounted BString implementation

Reported by: jackburton Owned by: axeld
Priority: low Milestone: Unscheduled
Component: - General Version:
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

Would be nice to have a reference counted, copy on write implementation for BString. Nowadays most string implementations are reference counted, so equal strings are only stored once, reducing the memory footprint.

Attachments (1)

BString.2.diff (81.8 KB) - added by julun 11 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 11 years ago by nielx

Could you please supply a diff?

comment:2 Changed 11 years ago by jackburton

I only had a quick look: looks good in theory, the only problem I can see is with binary compatibility, since you replaced the char& operator[] with a BStringRef operator[].

I can understand why you've done this, but this will break binary compatibility.

comment:3 in reply to:  2 Changed 11 years ago by julun

Hi,

I replace the char& operator only in case when build for target HAIKU, are we forced to stay binary compatible then as well?

Regards, Julun

comment:4 Changed 11 years ago by nielx

Yes. Binary compatibility means that BeOS apps can run on Haiku. If the interface of the Haiku string is different then the one on BeOS, an application for BeOS will not work.

comment:5 Changed 11 years ago by julun

Well then, i disabled the code for the moment. See attached diff and thx for looking into it.

comment:6 Changed 11 years ago by julun

We might consider to enable the code for gcc4, as this will break bin compat anyway. To the user BStringRef is seen as char anyway, so no code change is needed.

comment:7 in reply to:  6 Changed 11 years ago by jackburton

Replying to julun:

We might consider to enable the code for gcc4, as this will break bin compat anyway. To the user BStringRef is seen as char anyway, so no code change is needed.

Yes, that's really a nice idea. Oh, another thing: Can't the BStringRef implementation be moved into the cpp file ? At least, I wouldnt' like to expose that much info to the header file.

Has anyone else any opinion about this patch ? To me it looks good from a code POV, although I haven't really tested it yet.

Changed 11 years ago by julun

Attachment: BString.2.diff added

comment:8 Changed 11 years ago by julun

Ok, i moved the implementation into the source file and added GNUC > 3 so that BStringRef will only be used with new builds. I have it running since some time on my system without any problems. thank you both for the comments so far :)

comment:9 Changed 11 years ago by jackburton

Resolution: fixed
Status: newclosed

I've applied the patch in hrev24102. Thank you!

Note: See TracTickets for help on using tickets.