IndexedString vs. implicit sharing

[Andreas Pakulat]

Hi,

On Wed, Oct 17, 2012 at 9:46 PM, Milian Wolff <mail at milianw.de> wrote:
> On Wednesday 17 October 2012 20:52:22 Andreas Pakulat wrote:
>> > But couldn't we use
>> > QString's in there directly?
>>
>> That would mean keeping all the bits/bytes for a QString in memory,
>> which was the whole reason for dropping their use.
>
> Right. I forgot that kind of. Probably makes the implementation of what I want
> even more complicated or even impossible. Let me give an example here, replace
> QString with KUrl to get the same problem:
>
>       QString qString;
>       qString.fill('.', 1000);
>       KDevelop::IndexedString indexedString(qString);
>       const int repeat = 10000;
>       QVector<QString> strings;
>       strings.resize(repeat);
>       for(int i = 0; i < repeat; ++i) {
>         strings[i] = indexedString.str();
>         QCOMPARE(qString, strings[i]);
>       }
>
> What do you expect that does memory wise? Well, it will hog up more and more
> memory here. That's the Use-Case I'd like to optimize if possible.

So the implicit sharing of QString fails here? Do you happen to know
why, since I don't understand how the implicit sharing works all that
well I'm just wondering wether there's a way to make it work in this
situation...

Also does this happen in the codebase without being able to change the
relevant code? I mean adding the same indexedstring's QString content
to a list multiple times looks like somethings wrong there...

>> > Maybe that would require us to make IndexedString
>> > only operate on QString's but I'd be all for that anyways to increase type
>> > safety. But well, food for though...
>>
>> What kind of type-safety do you gain by replacing IndexedString with
>> QString?
>
> IndexedString has .str() .c_str() .byteArray() and finally .toUrl() - which
> one is the correct one?
>
> DUContext::url() -> should always be a KUrl
> Declaration::identifier() -> should alays be a QString
>
> That's what I meant with "type-safety" - probably the wrong word.

Ok, then rename IndexString to IndexObject, make the constructor
private and drop those 4 functions from it. Create 4 subclasses:
IndexedUrl, IndexedQString, IndexedByteArray and IndexedCString which
each have one of those 4 functions in their public API. Then change
all the code to use the suitable Index* class instead of
IndexedString.

Seeing the c-string and bytearray conversion immediately triggers my
encoding-alarm, how does IndexedString decide which encoding to use
and are all callers of these two function aware of the encoding being
used?

Andreas