immutable strings - approved
Allison Randal
through the branch diff (immutable_strings_part1). It's quite impressive
how much redundant code gets removed when we can rely on a string to
never change.
The branch does a good job of hiding the details from the user too. That
is, for the most part HLL code can run ahead as usual, and never need to
know that a given string operation has changed a register/variable to
point to a different string header.
A few comments:
* Since opcodes like 'downcase' are changing to return a value rather
than modifying one, then the opcodes should be clear about it. That is,
deprecate the one argument:
downcase $S0
In favor of the two argument:
$S1 = downcase $S0
* We've lost the semantic difference between 'set' and 'assign' for
strings, they're now identical. This isn't necessarily a bad thing, but
it seems sensible to deprecate 'assign' for strings and keep it only for
PMCs where it's meaningful.
* In src/string/encoding/fixed_8.c there's a stray FIXME in the wrong
comment style (// instead of /* */) which I'm guessing someone intends
to remove before merging.
* In src/string/api.c, does 'string_capacity' really make sense anymore
as a function? Shouldn't the capacity of an immutable string always be
the same as the length of the string? Possibly another deprecation item.
* Can Parrot_str_append be changed to just call Parrot_str_concat? Or,
perhaps deprecate append, since they notionally do the same thing now.
* It sure removes a lot of cruft from substr to have it only return a
new value.
* Another stray wrong-style comment in src/string/api.c, in
Parrot_str_pin on a call to Parrot_str_write_COW. Again on another call
to the same function later in the same file.
* A bit of complexity added to join, but unavoidable with immutable
strings. At least it avoids generating a pile of temporary strings by
allocating enough memory to hold the whole result at once.
Three cheers for unconventional thinking, finding speed gains and code
simplifications in unexpected places.
Allison
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev
Vasily Chekalkin
On Thu, Apr 15, 2010 at 9:36 AM, Allison Randal <all...@parrot.org> wrote:
> A few comments:
>
> * Since opcodes like 'downcase' are changing to return a value rather than
> modifying one, then the opcodes should be clear about it. That is, deprecate
> the one argument:
>
> downcase $S0
>
> In favor of the two argument:
>
> $S1 = downcase $S0
Yes. It's even in deprecated.pod already. But it's out of scope for
this branch. Main focus of branch is make immutable string works (and
that's why it has "part1" suffix").
> * We've lost the semantic difference between 'set' and 'assign' for strings,
> they're now identical. This isn't necessarily a bad thing, but it seems
> sensible to deprecate 'assign' for strings and keep it only for PMCs where
> it's meaningful.
Good idea.
> * In src/string/encoding/fixed_8.c there's a stray FIXME in the wrong
> comment style (// instead of /* */) which I'm guessing someone intends to
> remove before merging.
Yes. It's special markers which can be easily caught by make codetest.
Will be definitely fixed before merge.
> * In src/string/api.c, does 'string_capacity' really make sense anymore as a
> function? Shouldn't the capacity of an immutable string always be the same
> as the length of the string? Possibly another deprecation item.
It's a bug... We shouldn't update string in-place in str_append. So
string_capacity will gone.
> * Can Parrot_str_append be changed to just call Parrot_str_concat? Or,
> perhaps deprecate append, since they notionally do the same thing now.
It's already changed. Currently str_concat is synonym for str_append.
We can deprecate one of this. Preferable "append" version because
"concat" is cleaner from semantic point of view.
> * Another stray wrong-style comment in src/string/api.c, in Parrot_str_pin
> on a call to Parrot_str_write_COW. Again on another call to the same
> function later in the same file.
Same as before.
> * A bit of complexity added to join, but unavoidable with immutable strings.
> At least it avoids generating a pile of temporary strings by allocating
> enough memory to hold the whole result at once.
Indeed. But we have to add more comments about join implementation.
Just because it's third implemented version and me and chromatic did a
few iterations on it to make it fast.
--
Bacek
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev
Andy Lester
On Apr 14, 2010, at 7:36 PM, Allison Randal wrote:
> I've become more enamored with the immutable strings idea as I read through the branch diff (immutable_strings_part1). It's quite impressive how much redundant code gets removed when we can rely on a string to never change.
As the official Conster of Parrot, I heartily endorse this change. I suspect that the compiler and/or splint will be able to do much magic with this much constness running around.
xoxo,
Andy
--
Andy Lester => an...@petdance.com => www.theworkinggeek.com => AIM:petdance
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev