Re: [webkit-dev] some WebCore::String plans
On Dec 20, 2007, at 10:23 AM, Marvin Decker wrote: I think this would have to be done incrementally to keep the patches sane. Sure, makes sense. But the total amount of change here isn't so great. Is it okay if code has a bunch of extra DepString<->String conversions while this process is happening? The code already has many; I'm not sure it's likely to get much worse. The key to this question is *where* the extra conversions are. I don't want to make a blanket statement that this is OK -- I think it's pretty easy to do this in a way where there's no one step that introduces a ton of additional conversion. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] some WebCore::String plans
On 12/19/07, Darin Adler <[EMAIL PROTECTED]> wrote: > > On Dec 19, 2007, at 12:53 PM, Marvin Decker wrote: > > > This sounds good. Are there any plan to replace DeprecatedString > > with String when this is complete? It would make a big difference. > > Yes, uses of DeprecatedString should all be removed. > > But that doesn't need to wait for this plan. Migration from > DeprecatedString can be done before, during, and/or after the things I > listed here. > > If you're working on replacing a particular use of DeprecatedString > with String and run into an inefficiency or problem then please raise > it on the list -- we should fix it right away rather than waiting. > > Is this something you're interested in working on? Heh, (hides under the desk). I am interested in KURL, maybe I will look into this. I think this would have to be done incrementally to keep the patches sane. Is it okay if code has a bunch of extra DepString<->String conversions while this process is happening? ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] some WebCore::String plans
On Dec 19, 2007, at 12:53 PM, Marvin Decker wrote: This sounds good. Are there any plan to replace DeprecatedString with String when this is complete? It would make a big difference. Yes, uses of DeprecatedString should all be removed. But that doesn't need to wait for this plan. Migration from DeprecatedString can be done before, during, and/or after the things I listed here. If you're working on replacing a particular use of DeprecatedString with String and run into an inefficiency or problem then please raise it on the list -- we should fix it right away rather than waiting. Is this something you're interested in working on? -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] some WebCore::String plans
This sounds good. Are there any plan to replace DeprecatedString with String when this is complete? It would make a big difference. Marvin On 12/18/07, Darin Adler <[EMAIL PROTECTED]> wrote: > > The WebCore::String class needs some work. I have some plans to improve > it. Here's an outline of what I have in mind. > > *immutability* > - eliminate all non-const functions from StringImpl; fixes tricky sharing > semantics > - change all uses of const StringImpl* to just plain StringImpl* because > there will no longer be any real difference between these types > - then, once RefCounted is made thread-safe, then we could remove the > copy() function altogether, since the only reason to use it would be thread > safety > > > *ownership* > - change functions that return StringImpl* to return > PassRefPtr instead, if they sometimes make a new object; makes > it harder to accidentally leak > - change functions that take StringImpl* to take PassRefPtr > instead if they take ownership; can reduce reference count churn > - make a release() function on String that returns a > PassRefPtr; can reduce reference count churn > > > *clarity* > - eliminate functions that return a StringImpl* from StringImpl, because > it's to easy to misunderstand and think these modify the string in place > - eliminate functions that return a String from String -- also easy to use > wrong for the same reasons > > > *performance* > - change TextStream into a class named StringBuilder for clients that are > building up strings (name inspired by Java); it will have get() and > release() functions that will return PassRefPtr > - add functions that take a PassRefPtr and return a > PassRefPtr for many string operations, since those can optimize > the "only one reference" case and re-use the same character buffer; change > the String class to use those as appropriate > - rename the String::impl() function to String::get() to match other > RefPtr classes > - consider eliminating String and using RefPtr at most or all > call sites > - consider changing call sites that take a const String& to instead take > StringImpl* or PassRefPtr > > > *names* > - rename StringImpl to SharedString > - consider renaming WebCore::String to StringRefPtr since it's a > lightweight wrapper around a RefPtr; one benefit is that it would allow us > to eliminate the header name PlatformString.h > > > These plans are still a bit rough. Please let me know what you think. > Depending on what feedback I get, I might make a Wiki page about this. > > > Some of this I will do soon. Some of it is longer-term. > > > -- Darin > > > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo/webkit-dev > > -- Best Regards, Marvin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] some WebCore::String plans
On Dec 18, 2007, at 6:19 PM, Maciej Stachowiak wrote: clarity - eliminate functions that return a StringImpl* from StringImpl, because it's to easy to misunderstand and think these modify the string in place - eliminate functions that return a String from String -- also easy to use wrong for the same reasons Where would we put functions that take a string and return a modified one? Seems like we'll still want those, if our string type becomes truly immutable. I was thinking they could be free functions that return a PassRefPtr. That seems like a wart -- not sure where these should go. performance - change TextStream into a class named StringBuilder for clients that are building up strings (name inspired by Java); it will have get() and release() functions that will return PassRefPtr I kind of like the += syntax for a StringBuilder better than the << syntax, but I guess that's harder to use if there's no operator+. I like += better too. We don't necessarily need to keep the << syntax. Or we could have both. It seems like the obvious implementation techniques are making a Vector and appending to it, or keeping a Vector, neither of those would play well with a non-releasing get(). I was thinking of Vector. The release() function would use the adopt function that takes a Vector. The get() is less important. It should probably be named copy() instead. - rename StringImpl to SharedString - consider renaming WebCore::String to StringRefPtr since it's a lightweight wrapper around a RefPtr; one benefit is that it would allow us to eliminate the header name PlatformString.h If you did both of those things, a better name for SharedString might be String. I think you are right. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] some WebCore::String plans
Mostly sounds good. On Dec 18, 2007, at 10:51 AM, Darin Adler wrote: The WebCore::String class needs some work. I have some plans to improve it. Here's an outline of what I have in mind. immutability - eliminate all non-const functions from StringImpl; fixes tricky sharing semantics - change all uses of const StringImpl* to just plain StringImpl* because there will no longer be any real difference between these types - then, once RefCounted is made thread-safe, then we could remove the copy() function altogether, since the only reason to use it would be thread safety ownership - change functions that return StringImpl* to return PassRefPtr instead, if they sometimes make a new object; makes it harder to accidentally leak - change functions that take StringImpl* to take PassRefPtr instead if they take ownership; can reduce reference count churn - make a release() function on String that returns a PassRefPtr; can reduce reference count churn clarity - eliminate functions that return a StringImpl* from StringImpl, because it's to easy to misunderstand and think these modify the string in place - eliminate functions that return a String from String -- also easy to use wrong for the same reasons Where would we put functions that take a string and return a modified one? Seems like we'll still want those, if our string type becomes truly immutable. performance - change TextStream into a class named StringBuilder for clients that are building up strings (name inspired by Java); it will have get() and release() functions that will return PassRefPtr I kind of like the += syntax for a StringBuilder better than the << syntax, but I guess that's harder to use if there's no operator+. It seems like the obvious implementation techniques are making a Vector and appending to it, or keeping a Vector, neither of those would play well with a non-releasing get(). - add functions that take a PassRefPtr and return a PassRefPtr for many string operations, since those can optimize the "only one reference" case and re-use the same character buffer; change the String class to use those as appropriate - rename the String::impl() function to String::get() to match other RefPtr classes - consider eliminating String and using RefPtr at most or all call sites - consider changing call sites that take a const String& to instead take StringImpl* or PassRefPtr names - rename StringImpl to SharedString - consider renaming WebCore::String to StringRefPtr since it's a lightweight wrapper around a RefPtr; one benefit is that it would allow us to eliminate the header name PlatformString.h If you did both of those things, a better name for SharedString might be String. These plans are still a bit rough. Please let me know what you think. Depending on what feedback I get, I might make a Wiki page about this. Some of this I will do soon. Some of it is longer-term. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] some WebCore::String plans
On Dec 18, 2007, at 10:56 AM, Geoffrey Garen wrote: Would you suggest the same kinds of changes to KJS::UString? Probably not. I think the issues with KJS::UString are different. Long term it would be nice to consider exactly what the differences are between two classes and how to reconcile them. The biggest difference is that while WebCore::String has no allowance for efficiently growing the string, which keeps it small (StringImpl is 12 bytes plus the character buffer), KJS::UString has optimizations for both growing the string and for substring sharing (KJS::UString::Rep is much larger than 12 bytes). One helpful step for reconciling the classes could be moving the WebCore::String family into WTF. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] some WebCore::String plans
Sounds good. Would you suggest the same kinds of changes to KJS::UString? Geoff On Dec 18, 2007, at 10:51 AM, Darin Adler wrote: The WebCore::String class needs some work. I have some plans to improve it. Here's an outline of what I have in mind. immutability - eliminate all non-const functions from StringImpl; fixes tricky sharing semantics - change all uses of const StringImpl* to just plain StringImpl* because there will no longer be any real difference between these types - then, once RefCounted is made thread-safe, then we could remove the copy() function altogether, since the only reason to use it would be thread safety ownership - change functions that return StringImpl* to return PassRefPtr instead, if they sometimes make a new object; makes it harder to accidentally leak - change functions that take StringImpl* to take PassRefPtr instead if they take ownership; can reduce reference count churn - make a release() function on String that returns a PassRefPtr; can reduce reference count churn clarity - eliminate functions that return a StringImpl* from StringImpl, because it's to easy to misunderstand and think these modify the string in place - eliminate functions that return a String from String -- also easy to use wrong for the same reasons performance - change TextStream into a class named StringBuilder for clients that are building up strings (name inspired by Java); it will have get() and release() functions that will return PassRefPtr - add functions that take a PassRefPtr and return a PassRefPtr for many string operations, since those can optimize the "only one reference" case and re-use the same character buffer; change the String class to use those as appropriate - rename the String::impl() function to String::get() to match other RefPtr classes - consider eliminating String and using RefPtr at most or all call sites - consider changing call sites that take a const String& to instead take StringImpl* or PassRefPtr names - rename StringImpl to SharedString - consider renaming WebCore::String to StringRefPtr since it's a lightweight wrapper around a RefPtr; one benefit is that it would allow us to eliminate the header name PlatformString.h These plans are still a bit rough. Please let me know what you think. Depending on what feedback I get, I might make a Wiki page about this. Some of this I will do soon. Some of it is longer-term. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] some WebCore::String plans
The WebCore::String class needs some work. I have some plans to improve it. Here's an outline of what I have in mind. immutability - eliminate all non-const functions from StringImpl; fixes tricky sharing semantics - change all uses of const StringImpl* to just plain StringImpl* because there will no longer be any real difference between these types - then, once RefCounted is made thread-safe, then we could remove the copy() function altogether, since the only reason to use it would be thread safety ownership - change functions that return StringImpl* to return PassRefPtr instead, if they sometimes make a new object; makes it harder to accidentally leak - change functions that take StringImpl* to take PassRefPtr instead if they take ownership; can reduce reference count churn - make a release() function on String that returns a PassRefPtr; can reduce reference count churn clarity - eliminate functions that return a StringImpl* from StringImpl, because it's to easy to misunderstand and think these modify the string in place - eliminate functions that return a String from String -- also easy to use wrong for the same reasons performance - change TextStream into a class named StringBuilder for clients that are building up strings (name inspired by Java); it will have get() and release() functions that will return PassRefPtr - add functions that take a PassRefPtr and return a PassRefPtr for many string operations, since those can optimize the "only one reference" case and re-use the same character buffer; change the String class to use those as appropriate - rename the String::impl() function to String::get() to match other RefPtr classes - consider eliminating String and using RefPtr at most or all call sites - consider changing call sites that take a const String& to instead take StringImpl* or PassRefPtr names - rename StringImpl to SharedString - consider renaming WebCore::String to StringRefPtr since it's a lightweight wrapper around a RefPtr; one benefit is that it would allow us to eliminate the header name PlatformString.h These plans are still a bit rough. Please let me know what you think. Depending on what feedback I get, I might make a Wiki page about this. Some of this I will do soon. Some of it is longer-term. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
