wren-lang / wren

The Wren Programming Language. Wren is a small, fast, class-based concurrent scripting language.
http://wren.io
MIT License
6.86k stars 550 forks source link

[RFC] Adding `const` versions of `Object`s. #1156

Open mhermier opened 1 year ago

mhermier commented 1 year ago

Hi, For reasons, some const versions of objects are required from time to time. I can think of at least 3 strategies:

edit: Removed the part of using it for controlling subclassing. It would make more sense if it was controlling mutation of static variables. But due to implementation details, it is not possible without heavy change in the Class implementation.

PureFox48 commented 1 year ago

Well, of the built-in classes, several are immutable already and, to my mind, the only other ones where it might be useful to have const versions are List and Map.

TupleConst which you've just proposed in #1151 would give us a const List and I don't think a const Map is worth the effort in core - if you really need it, it's easy to write your own.

So I think I'd be very much in the 'do nothing' camp here and leave it to library writers or other users to create const versions of their classes if they think they'd be useful.

Sealing classes is, of course, a different matter to immutability and most of the built-in classes are implicitly sealed for good reason. In general, though, I'd be against sealing user defined classes as my experience with other languages which allow this is that it can be a nuisance in practice and people will try to find ways around it.

I didn't follow your point about controlling mutation of static variables as they aren't inheritable (at present anyway) and you can control access to them by just having a getter but no setter.

mhermier commented 1 year ago

While I understand your points, a TupleConst while providing almost similar functions to a const List differs from it in 2 critical points (I conserve the example here but I consider it a good representative for the general case):

As of today, the static variables (__foo) are stored within the module data, and not in the class. It means the isConstant flag would have no effect, since the access to these variable are translated to direct access to the module variables stack.

PureFox48 commented 1 year ago

I'm just throwing ideas around here but one answer to this would be to have a ConstList and interpose that in the inheritance chain from Sequence. So both List and Tuple would inherit from ConstList and thereby avoid a fair bit of code duplication.

mhermier commented 1 year ago

If interface become a concrete thing, most functionality could go in there, but that would not solve hierarchy either. Will look if there is precedence somewhere in one of the languages...

mhermier commented 1 year ago

One solution from some java developer: using attributes is to create an ImmutableProxy automagically. It makes the process more straight forward.

PureFox48 commented 1 year ago

That's a possibility though currently attributes in Wren are just user-defined so we'd need to extend that to allow built-in ones.

Javascript has an Object.freeze method but I don't think that would be practicable in Wren. Every object would need to have a boolean isFrozen field and methods which attempted to mutate that object would need to check that field to see if it were allowable.

mhermier commented 1 year ago

Seems there is also prior art in Pharo Smalltalk with isReadOnlyObject:.

In practice, it should only need to be checked for primitives/foreign, and for static methods (because of implementation details). We can automatically intercept writes to instances in STORE_FIELD_THIS STORE_FIELD opcodes. I'll give it a try using java naming (unless there is a better name). I want to see how much it does impact performances, out of curriosity.

mhermier commented 1 year ago

Made a basic implementation. I still have to add test to ensure it works properly before I can publish something more seriously. Thought the initial penalty is quite "low".

api_call - wren                .......... 0.10s 0.0021  93.99% relative to baseline
api_foreign_method - wren      .......... 0.58s 0.0078  99.56% relative to baseline
binary_trees - wren            .......... 0.34s 0.0047  94.96% relative to baseline
binary_trees_gc - wren         .......... 1.33s 0.0079  96.51% relative to baseline
delta_blue - wren              .......... 0.29s 0.0039 101.85% relative to baseline
fib - wren                     .......... 0.40s 0.0123  99.48% relative to baseline
fibers - wren                  .......... 0.06s 0.0028  98.51% relative to baseline
for - wren                     .......... 0.16s 0.0025 100.39% relative to baseline
method_call - wren             .......... 0.22s 0.0048 108.67% relative to baseline
map_numeric - wren             .......... 1.28s 0.0133 100.04% relative to baseline
map_string - wren              .......... 0.14s 0.0012  99.24% relative to baseline
string_equals - wren           .......... 0.27s 0.0048  99.24% relative to baseline

That said, I made a quick check about likely and unlikely (__builtin_expect) branch prediction. We really need to investigate for inclusion. After adding some in the hot simulation loop on top of the previous change:

api_call - wren                .......... 0.10s 0.0041 100.76% relative to baseline
api_foreign_method - wren      .......... 0.41s 0.0017 140.66% relative to baseline
binary_trees - wren            .......... 0.30s 0.0037 106.91% relative to baseline
binary_trees_gc - wren         .......... 1.30s 0.0043  98.56% relative to baseline
delta_blue - wren              .......... 0.29s 0.0028 102.53% relative to baseline
fib - wren                     .......... 0.38s 0.0021 106.60% relative to baseline
fibers - wren                  .......... 0.06s 0.0037 100.27% relative to baseline
for - wren                     .......... 0.16s 0.0039 101.67% relative to baseline
method_call - wren             .......... 0.21s 0.0052 114.42% relative to baseline
map_numeric - wren             .......... 1.28s 0.0217  99.86% relative to baseline
map_string - wren              .......... 0.14s 0.0014 100.98% relative to baseline
string_equals - wren           .......... 0.26s 0.0077 100.42% relative to baseline
PureFox48 commented 1 year ago

Better than I expected. Might be a viable approach after all :)