ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:99958] [Ruby master Bug#17159] extend `define_method` for Ractor
@ 2020-09-07  1:49 ko1
  2020-09-07  2:39 ` [ruby-core:99959] " matz
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: ko1 @ 2020-09-07  1:49 UTC (permalink / raw)
  To: ruby-core

Issue #17159 has been reported by ko1 (Koichi Sasada).

----------------------------------------
Bug #17159: extend `define_method` for Ractor
https://bugs.ruby-lang.org/issues/17159

* Author: ko1 (Koichi Sasada)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
Ractor prohibits to use non-isolated Procs.

Non-isolated example is here:

```
s = "foo"
pr = Proc.new{ p s }
```

This Proc pr can not be shared with multi-ractors because outer variable `s` can contain an unshareable object. Also outer binding is mutable object so it can lead race condition.

Because of these reasons, `define_method` are also problem on multi-Ractor program.
(current implementation allows it just because check is not implemented, and it leads BUG).


I think there are several patterns `define_method` are used.

(1) want to choose variable method names

```ruby
name = ...
define_method(name){ nil }
```

(2) want to embed variables to the code

```ruby
10.times{|i|
  define_method("foo{i}"){ i }
}
```

(3) want to use global state by local variables

```ruby
cnt = 0
define_method("inc"){ cnt += 1 }
```

(4) others I can't imagine

----

(1) is easy. `define_method(name, &Proc{nil}.isoplate)` will be allowed on multi-ractors.

(3) is not allowed because it introduces data races/race conditions. For example, we need to use shared hash.

```ruby
STATE = SharedHash.new(cnt: 0)
define_method("inc"){ STATE.transaction{ STATE[:cnt] += 1 }}
```

I think there are many (2) patterns and it should be saved.
To help (2) pattern, the easiest way is to use eval.

```ruby
10.times{|i|
  eval("def foo#{i} #{i}; end")
}

```

However, eval has several issues (it has huge freedom to explode the program, editor's syntax highlighting and so on).

Another approach is embed the current value to the code, like that:


```ruby
i = 0
define_method("foo", ractorise: true){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", ractorise: true){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

However, it is very difference between current Proc semantics.
Another idea is to specify embedding value like that.

```ruby
i = 0
define_method("foo", i: i){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", s: s){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

`i: i` and `s: s` are redundant. however, if there are no outer variable `i` or `s`, the `i` and `s` in blocks are compiled to `send(:i)` or `send(:s)`. But I agree these method invocation should be replaced is another idea.


Thoughts?

Thanks,
Koichi



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ruby-core:99959] [Ruby master Bug#17159] extend `define_method` for Ractor
  2020-09-07  1:49 [ruby-core:99958] [Ruby master Bug#17159] extend `define_method` for Ractor ko1
@ 2020-09-07  2:39 ` matz
  2020-09-07  4:36 ` [ruby-core:99960] " ko1
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: matz @ 2020-09-07  2:39 UTC (permalink / raw)
  To: ruby-core

Issue #17159 has been updated by matz (Yukihiro Matsumoto).


Just a comment. In general, 

```ruby
i = 0
define_method("foo{i}"){ i }
```

and

```ruby
i = 0
eval("def foo#{i} #{i}; end")
```

behave differently. The former returns the current value of `i` in the closure, the latter embed the value of `i` when the method is defined.
In the above example, they behave same because `i` is a block local variable and not shared by other closures

Matz.

----------------------------------------
Bug #17159: extend `define_method` for Ractor
https://bugs.ruby-lang.org/issues/17159#change-87498

* Author: ko1 (Koichi Sasada)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
Ractor prohibits to use non-isolated Procs.

Non-isolated example is here:

```
s = "foo"
pr = Proc.new{ p s }
```

This Proc pr can not be shared with multi-ractors because outer variable `s` can contain an unshareable object. Also outer binding is mutable object so it can lead race condition.

Because of these reasons, `define_method` are also problem on multi-Ractor program.
(current implementation allows it just because check is not implemented, and it leads BUG).


I think there are several patterns `define_method` are used.

(1) want to choose variable method names

```ruby
name = ...
define_method(name){ nil }
```

(2) want to embed variables to the code

```ruby
10.times{|i|
  define_method("foo{i}"){ i }
}
```

(3) want to use global state by local variables

```ruby
cnt = 0
define_method("inc"){ cnt += 1 }
```

(4) others I can't imagine

----

(1) is easy. `define_method(name, &Proc{nil}.isoplate)` will be allowed on multi-ractors.

(3) is not allowed because it introduces data races/race conditions. For example, we need to use shared hash.

```ruby
STATE = SharedHash.new(cnt: 0)
define_method("inc"){ STATE.transaction{ STATE[:cnt] += 1 }}
```

I think there are many (2) patterns and it should be saved.
To help (2) pattern, the easiest way is to use eval.

```ruby
10.times{|i|
  eval("def foo#{i} #{i}; end")
}

```

However, eval has several issues (it has huge freedom to explode the program, editor's syntax highlighting and so on).

Another approach is embed the current value to the code, like that:


```ruby
i = 0
define_method("foo", ractorise: true){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", ractorise: true){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

However, it is very difference between current Proc semantics.
Another idea is to specify embedding value like that.

```ruby
i = 0
define_method("foo", i: i){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", s: s){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

`i: i` and `s: s` are redundant. however, if there are no outer variable `i` or `s`, the `i` and `s` in blocks are compiled to `send(:i)` or `send(:s)`. But I agree these method invocation should be replaced is another idea.


Thoughts?

Thanks,
Koichi



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ruby-core:99960] [Ruby master Bug#17159] extend `define_method` for Ractor
  2020-09-07  1:49 [ruby-core:99958] [Ruby master Bug#17159] extend `define_method` for Ractor ko1
  2020-09-07  2:39 ` [ruby-core:99959] " matz
@ 2020-09-07  4:36 ` ko1
  2020-09-13 10:01 ` [ruby-core:100002] " eregontp
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: ko1 @ 2020-09-07  4:36 UTC (permalink / raw)
  To: ruby-core

Issue #17159 has been updated by ko1 (Koichi Sasada).


Yes, I mean most of case (2) can be replaced with `eval` and proposed changes.


----------------------------------------
Bug #17159: extend `define_method` for Ractor
https://bugs.ruby-lang.org/issues/17159#change-87501

* Author: ko1 (Koichi Sasada)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
Ractor prohibits to use non-isolated Procs.

Non-isolated example is here:

```
s = "foo"
pr = Proc.new{ p s }
```

This Proc pr can not be shared with multi-ractors because outer variable `s` can contain an unshareable object. Also outer binding is mutable object so it can lead race condition.

Because of these reasons, `define_method` are also problem on multi-Ractor program.
(current implementation allows it just because check is not implemented, and it leads BUG).


I think there are several patterns `define_method` are used.

(1) want to choose variable method names

```ruby
name = ...
define_method(name){ nil }
```

(2) want to embed variables to the code

```ruby
10.times{|i|
  define_method("foo{i}"){ i }
}
```

(3) want to use global state by local variables

```ruby
cnt = 0
define_method("inc"){ cnt += 1 }
```

(4) others I can't imagine

----

(1) is easy. `define_method(name, &Proc{nil}.isoplate)` will be allowed on multi-ractors.

(3) is not allowed because it introduces data races/race conditions. For example, we need to use shared hash.

```ruby
STATE = SharedHash.new(cnt: 0)
define_method("inc"){ STATE.transaction{ STATE[:cnt] += 1 }}
```

I think there are many (2) patterns and it should be saved.
To help (2) pattern, the easiest way is to use eval.

```ruby
10.times{|i|
  eval("def foo#{i} #{i}; end")
}

```

However, eval has several issues (it has huge freedom to explode the program, editor's syntax highlighting and so on).

Another approach is embed the current value to the code, like that:


```ruby
i = 0
define_method("foo", ractorise: true){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", ractorise: true){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

However, it is very difference between current Proc semantics.
Another idea is to specify embedding value like that.

```ruby
i = 0
define_method("foo", i: i){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", s: s){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

`i: i` and `s: s` are redundant. however, if there are no outer variable `i` or `s`, the `i` and `s` in blocks are compiled to `send(:i)` or `send(:s)`. But I agree these method invocation should be replaced is another idea.


Thoughts?

Thanks,
Koichi



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ruby-core:100002] [Ruby master Bug#17159] extend `define_method` for Ractor
  2020-09-07  1:49 [ruby-core:99958] [Ruby master Bug#17159] extend `define_method` for Ractor ko1
  2020-09-07  2:39 ` [ruby-core:99959] " matz
  2020-09-07  4:36 ` [ruby-core:99960] " ko1
@ 2020-09-13 10:01 ` eregontp
  2020-09-13 10:07 ` [ruby-core:100003] " eregontp
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: eregontp @ 2020-09-13 10:01 UTC (permalink / raw)
  To: ruby-core

Issue #17159 has been updated by Eregon (Benoit Daloze).


Copying captured variables seems a nice feature to have, also for optimizations.
In fact, this already exists in TruffleRuby internally:
https://github.com/oracle/truffleruby/blob/574d6bd2425caa856707ffd713fdb8ffc87be1e1/src/main/java/org/truffleruby/extra/TruffleGraalNodes.java#L113-L120
It seems known as `fixTemps` in Smalltalk.

So I'd suggest to add this as a keyword argument, not sure about the name but `ractorize` doesn't explain what it does.
`copy_captured_locals: true`, `copy_captured_variables: true`, `capture_scope: false` maybe?

This can also be useful on `Proc` in general, so it might be better to have a method on `Proc` for that.
Having it as a keyword argument for `define_method` as a shortcut seems good, since on the implementation side it's useful to combine both operations into one to avoid intermediate Procs, bytecode, etc.

For Ractor there is a big limitation though: only shareable objects can be copied that way.
Otherwise it would lead to shared state and segv.

----------------------------------------
Bug #17159: extend `define_method` for Ractor
https://bugs.ruby-lang.org/issues/17159#change-87549

* Author: ko1 (Koichi Sasada)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
Ractor prohibits to use non-isolated Procs.

Non-isolated example is here:

```
s = "foo"
pr = Proc.new{ p s }
```

This Proc pr can not be shared with multi-ractors because outer variable `s` can contain an unshareable object. Also outer binding is mutable object so it can lead race condition.

Because of these reasons, `define_method` are also problem on multi-Ractor program.
(current implementation allows it just because check is not implemented, and it leads BUG).


I think there are several patterns `define_method` are used.

(1) want to choose variable method names

```ruby
name = ...
define_method(name){ nil }
```

(2) want to embed variables to the code

```ruby
10.times{|i|
  define_method("foo{i}"){ i }
}
```

(3) want to use global state by local variables

```ruby
cnt = 0
define_method("inc"){ cnt += 1 }
```

(4) others I can't imagine

----

(1) is easy. `define_method(name, &Proc{nil}.isoplate)` will be allowed on multi-ractors.

(3) is not allowed because it introduces data races/race conditions. For example, we need to use shared hash.

```ruby
STATE = SharedHash.new(cnt: 0)
define_method("inc"){ STATE.transaction{ STATE[:cnt] += 1 }}
```

I think there are many (2) patterns and it should be saved.
To help (2) pattern, the easiest way is to use eval.

```ruby
10.times{|i|
  eval("def foo#{i} #{i}; end")
}

```

However, eval has several issues (it has huge freedom to explode the program, editor's syntax highlighting and so on).

Another approach is embed the current value to the code, like that:


```ruby
i = 0
define_method("foo", ractorise: true){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", ractorise: true){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

However, it is very difference between current Proc semantics.
Another idea is to specify embedding value like that.

```ruby
i = 0
define_method("foo", i: i){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", s: s){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

`i: i` and `s: s` are redundant. however, if there are no outer variable `i` or `s`, the `i` and `s` in blocks are compiled to `send(:i)` or `send(:s)`. But I agree these method invocation should be replaced is another idea.


Thoughts?

Thanks,
Koichi



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ruby-core:100003] [Ruby master Bug#17159] extend `define_method` for Ractor
  2020-09-07  1:49 [ruby-core:99958] [Ruby master Bug#17159] extend `define_method` for Ractor ko1
                   ` (2 preceding siblings ...)
  2020-09-13 10:01 ` [ruby-core:100002] " eregontp
@ 2020-09-13 10:07 ` eregontp
  2020-09-14  1:29 ` [ruby-core:100004] " shyouhei
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: eregontp @ 2020-09-13 10:07 UTC (permalink / raw)
  To: ruby-core

Issue #17159 has been updated by Eregon (Benoit Daloze).


> #eval (...) has huge freedom to explode the program

I think this will happen too with the new feature proposed here.
One needs new bytecode/etc for the Proc/method where `i` is replaced by a literal value, isn't it?
Or do you alternatively copy the captured frame?

Regarding implementation, this also needs to handle nested blocks inside `define_method`.


----------------------------------------
Bug #17159: extend `define_method` for Ractor
https://bugs.ruby-lang.org/issues/17159#change-87552

* Author: ko1 (Koichi Sasada)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
Ractor prohibits to use non-isolated Procs.

Non-isolated example is here:

```
s = "foo"
pr = Proc.new{ p s }
```

This Proc pr can not be shared with multi-ractors because outer variable `s` can contain an unshareable object. Also outer binding is mutable object so it can lead race condition.

Because of these reasons, `define_method` are also problem on multi-Ractor program.
(current implementation allows it just because check is not implemented, and it leads BUG).


I think there are several patterns `define_method` are used.

(1) want to choose variable method names

```ruby
name = ...
define_method(name){ nil }
```

(2) want to embed variables to the code

```ruby
10.times{|i|
  define_method("foo{i}"){ i }
}
```

(3) want to use global state by local variables

```ruby
cnt = 0
define_method("inc"){ cnt += 1 }
```

(4) others I can't imagine

----

(1) is easy. `define_method(name, &Proc{nil}.isoplate)` will be allowed on multi-ractors.

(3) is not allowed because it introduces data races/race conditions. For example, we need to use shared hash.

```ruby
STATE = SharedHash.new(cnt: 0)
define_method("inc"){ STATE.transaction{ STATE[:cnt] += 1 }}
```

I think there are many (2) patterns and it should be saved.
To help (2) pattern, the easiest way is to use eval.

```ruby
10.times{|i|
  eval("def foo#{i} #{i}; end")
}

```

However, eval has several issues (it has huge freedom to explode the program, editor's syntax highlighting and so on).

Another approach is embed the current value to the code, like that:


```ruby
i = 0
define_method("foo", ractorise: true){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", ractorise: true){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

However, it is very difference between current Proc semantics.
Another idea is to specify embedding value like that.

```ruby
i = 0
define_method("foo", i: i){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", s: s){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

`i: i` and `s: s` are redundant. however, if there are no outer variable `i` or `s`, the `i` and `s` in blocks are compiled to `send(:i)` or `send(:s)`. But I agree these method invocation should be replaced is another idea.


Thoughts?

Thanks,
Koichi



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ruby-core:100004] [Ruby master Bug#17159] extend `define_method` for Ractor
  2020-09-07  1:49 [ruby-core:99958] [Ruby master Bug#17159] extend `define_method` for Ractor ko1
                   ` (3 preceding siblings ...)
  2020-09-13 10:07 ` [ruby-core:100003] " eregontp
@ 2020-09-14  1:29 ` shyouhei
  2020-10-25 20:08 ` [ruby-core:100538] " marcandre-ruby-core
  2020-10-29 16:06 ` [ruby-core:100646] " ko1
  6 siblings, 0 replies; 8+ messages in thread
From: shyouhei @ 2020-09-14  1:29 UTC (permalink / raw)
  To: ruby-core

Issue #17159 has been updated by shyouhei (Shyouhei Urabe).

Description updated

### The reason I use `#define_method` (4)

I sometimes use it to alias _a part_ of a module, like this:

```ruby
class Foo
  %i[sin cos tan].each do |sym|
    define_method(sym, Math.instance_method(sym))
  end
end
p Foo.new.sin(3.14)
```

There seems be no reason to reject such usages.

### Capturing local variables

C++ since C++11 have had lambdas.  In the language you can explicitly specify how you want to capture a variable each time when you create a lambda.  As of C++20 there are 12 different specifier.  Some of them exist for template metaprogramming (we can ignore such things), but I think there are several interesting cases.

```C++
#include <cstdio>

int main() {
    int x, y, z;

    x = y = z = 1;

    auto f = [x, &y, &z]() mutable {
        auto g = [x, y, &z]() mutable {
            printf("#1: x, y, z = %d, %d, %d\n", x, y, z);
            x = y = z = 4;
        };

        x = y = z = 3;
        g();
    };

    x = y = z = 2;
    f();

    printf("#2: x, y, z = %d, %d, %d\n", x, y, z);
}
```

This program outputs

```
#1: x, y, z = 1, 2, 3
#2: x, y, z = 2, 3, 4
```

Compilicated?  But the `s: s` proposal is very much like this.  You can mix call-by-reference and call-by-value.  I agree this gives us maximum freedom, but at a cost of complexity.

C++ also has simpler specifier which has no such headaches:

```C++
#include <cstdio>

int main() {
    int x, y, z;

    x = y = z = 1;

    auto f = [=]() mutable {
        auto g = [=]() mutable {
            printf("#1: x, y, z = %d, %d, %d\n", x, y, z);
            x = y = z = 4;
        };

        x = y = z = 3;
        g();
    };

    x = y = z = 2;
    f();

    printf("#2: x, y, z = %d, %d, %d\n", x, y, z);
}
```

The above should print:

```
#1: x, y, z = 1, 1, 1
#2: x, y, z = 2, 2, 2
```

And I think this behaviour is much more understandable.  The `ractorise: true` proposal is on this line.  I'd push this way.

----------------------------------------
Bug #17159: extend `define_method` for Ractor
https://bugs.ruby-lang.org/issues/17159#change-87553

* Author: ko1 (Koichi Sasada)
* Status: Open
* Priority: Normal
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
Ractor prohibits use of non-isolated `Proc`s.

Non-isolated example is here:

```ruby
s = "foo"
pr = Proc.new{ p s }
```

This Proc `pr` can not be shared among ractors because outer variable `s` can contain an unshareable object. Also outer binding is a mutable object. Sharing it can lead race conditions.

Because of these reasons, `define_method` is also a problem on a multi-Ractor program.
(current implementation allows it just because check is not implemented, and it leads BUG).


I think there are several patterns when `define_method` is needed.

(1) To choose method names on-the-fly

```ruby
name = ...
define_method(name){ nil }
```

(2) To embed variables to the code

```ruby
10.times{|i|
  define_method("foo{i}"){ i }
}
```

(3) To use global state by local variables

```ruby
cnt = 0
define_method("inc"){ cnt += 1 }
```

(4) Others I can't imagine

----

(1) is easy. We can allow `define_method(name, &Proc{nil}.isoplate)`.

(3) can never be OK. It introduces data races/race conditions. For this purpose one need to use shared hash.

```ruby
STATE = SharedHash.new(cnt: 0)
define_method("inc"){ STATE.transaction{ STATE[:cnt] += 1 }}
```

I think there are many (2) patterns that should be saved.
To help (2) pattern, the easiest way is to use `eval`.

```ruby
10.times{|i|
  eval("def foo#{i} #{i}; end")
}
```

However, `eval` has several issues (it has huge freedom to explode the program, editor's syntax highlighting and so on).

Another approach is to embed the current value to the code, like this:

```ruby
i = 0
define_method("foo", ractorise: true){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", ractorise: true){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

However, it is very differenct from current Proc semantics.

Another idea is to specify embedding value like this:

```ruby
i = 0
define_method("foo", i: i){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", s: s){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

`i: i` and `s: s` are redundant. However, if there are no outer variable `i` or `s`, the `i` and `s` in blocks are compiled to `send(:i)` or `send(:s)`. But I agree these method invocation should be replaced is another idea.


Thoughts?

Thanks,
Koichi



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ruby-core:100538] [Ruby master Bug#17159] extend `define_method` for Ractor
  2020-09-07  1:49 [ruby-core:99958] [Ruby master Bug#17159] extend `define_method` for Ractor ko1
                   ` (4 preceding siblings ...)
  2020-09-14  1:29 ` [ruby-core:100004] " shyouhei
@ 2020-10-25 20:08 ` marcandre-ruby-core
  2020-10-29 16:06 ` [ruby-core:100646] " ko1
  6 siblings, 0 replies; 8+ messages in thread
From: marcandre-ruby-core @ 2020-10-25 20:08 UTC (permalink / raw)
  To: ruby-core

Issue #17159 has been updated by marcandre (Marc-Andre Lafortune).


How about:

```ruby
define_method(:name, make_shareable: true) { ... }
# equivalent to:
define_method(:name, &Ractor.make_shareable(Proc.new{...}))`
```

With `make_shareable` as making accessed external variables shareable and not reassignable.

----------------------------------------
Bug #17159: extend `define_method` for Ractor
https://bugs.ruby-lang.org/issues/17159#change-88166

* Author: ko1 (Koichi Sasada)
* Status: Open
* Priority: Normal
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: DONTNEED
----------------------------------------
Ractor prohibits use of non-isolated `Proc`s.

Non-isolated example is here:

```ruby
s = "foo"
pr = Proc.new{ p s }
```

This Proc `pr` can not be shared among ractors because outer variable `s` can contain an unshareable object. Also outer binding is a mutable object. Sharing it can lead race conditions.

Because of these reasons, `define_method` is also a problem on a multi-Ractor program.
(current implementation allows it just because check is not implemented, and it leads BUG).


I think there are several patterns when `define_method` is needed.

(1) To choose method names on-the-fly

```ruby
name = ...
define_method(name){ nil }
```

(2) To embed variables to the code

```ruby
10.times{|i|
  define_method("foo#{i}"){ i }
}
```

(3) To use global state by local variables

```ruby
cnt = 0
define_method("inc"){ cnt += 1 }
```

(4) Others I can't imagine

----

(1) is easy. We can allow `define_method(name, &Proc{nil}.isolate)`.

(3) can never be OK. It introduces data races/race conditions. For this purpose one need to use shared hash.

```ruby
STATE = SharedHash.new(cnt: 0)
define_method("inc"){ STATE.transaction{ STATE[:cnt] += 1 }}
```

I think there are many (2) patterns that should be saved.
To help (2) pattern, the easiest way is to use `eval`.

```ruby
10.times{|i|
  eval("def foo#{i} #{i}; end")
}
```

However, `eval` has several issues (it has huge freedom to explode the program, editor's syntax highlighting and so on).

Another approach is to embed the current value to the code, like this:

```ruby
i = 0
define_method("foo", ractorise: true){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", ractorise: true){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

However, it is very differenct from current Proc semantics.

Another idea is to specify embedding value like this:

```ruby
i = 0
define_method("foo", i: i){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", s: s){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

`i: i` and `s: s` are redundant. However, if there are no outer variable `i` or `s`, the `i` and `s` in blocks are compiled to `send(:i)` or `send(:s)`. But I agree these method invocation should be replaced is another idea.


Thoughts?

Thanks,
Koichi



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ruby-core:100646] [Ruby master Bug#17159] extend `define_method` for Ractor
  2020-09-07  1:49 [ruby-core:99958] [Ruby master Bug#17159] extend `define_method` for Ractor ko1
                   ` (5 preceding siblings ...)
  2020-10-25 20:08 ` [ruby-core:100538] " marcandre-ruby-core
@ 2020-10-29 16:06 ` ko1
  6 siblings, 0 replies; 8+ messages in thread
From: ko1 @ 2020-10-29 16:06 UTC (permalink / raw)
  To: ruby-core

Issue #17159 has been updated by ko1 (Koichi Sasada).


marcandre (Marc-Andre Lafortune) wrote in #note-8:
> How about:
> 
> ```ruby
> define_method(:name, make_shareable: true) { ... }
> # equivalent to:
> define_method(:name, &Ractor.make_shareable(Proc.new{...}))`
> ```

Matz, how about this proposal?


----------------------------------------
Bug #17159: extend `define_method` for Ractor
https://bugs.ruby-lang.org/issues/17159#change-88284

* Author: ko1 (Koichi Sasada)
* Status: Open
* Priority: Normal
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: DONTNEED
----------------------------------------
Ractor prohibits use of non-isolated `Proc`s.

Non-isolated example is here:

```ruby
s = "foo"
pr = Proc.new{ p s }
```

This Proc `pr` can not be shared among ractors because outer variable `s` can contain an unshareable object. Also outer binding is a mutable object. Sharing it can lead race conditions.

Because of these reasons, `define_method` is also a problem on a multi-Ractor program.
(current implementation allows it just because check is not implemented, and it leads BUG).


I think there are several patterns when `define_method` is needed.

(1) To choose method names on-the-fly

```ruby
name = ...
define_method(name){ nil }
```

(2) To embed variables to the code

```ruby
10.times{|i|
  define_method("foo#{i}"){ i }
}
```

(3) To use global state by local variables

```ruby
cnt = 0
define_method("inc"){ cnt += 1 }
```

(4) Others I can't imagine

----

(1) is easy. We can allow `define_method(name, &Proc{nil}.isolate)`.

(3) can never be OK. It introduces data races/race conditions. For this purpose one need to use shared hash.

```ruby
STATE = SharedHash.new(cnt: 0)
define_method("inc"){ STATE.transaction{ STATE[:cnt] += 1 }}
```

I think there are many (2) patterns that should be saved.
To help (2) pattern, the easiest way is to use `eval`.

```ruby
10.times{|i|
  eval("def foo#{i} #{i}; end")
}
```

However, `eval` has several issues (it has huge freedom to explode the program, editor's syntax highlighting and so on).

Another approach is to embed the current value to the code, like this:

```ruby
i = 0
define_method("foo", ractorise: true){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", ractorise: true){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

However, it is very differenct from current Proc semantics.

Another idea is to specify embedding value like this:

```ruby
i = 0
define_method("foo", i: i){ i }
#=> equivalent to:
#   define_method("foo"){ 0 }
# so that if outer scope's i changed, not affected.
i = 1
foo #=> 0

s = ""
define_method("bar", s: s){ s }
#=> equivalent to:
#   define_method("bar"){ "" }
# so that if outer scope's s or s's value, it doesn't affect
s << "x"
bar #=> ""
```

`i: i` and `s: s` are redundant. However, if there are no outer variable `i` or `s`, the `i` and `s` in blocks are compiled to `send(:i)` or `send(:s)`. But I agree these method invocation should be replaced is another idea.


Thoughts?

Thanks,
Koichi



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-10-29 16:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-07  1:49 [ruby-core:99958] [Ruby master Bug#17159] extend `define_method` for Ractor ko1
2020-09-07  2:39 ` [ruby-core:99959] " matz
2020-09-07  4:36 ` [ruby-core:99960] " ko1
2020-09-13 10:01 ` [ruby-core:100002] " eregontp
2020-09-13 10:07 ` [ruby-core:100003] " eregontp
2020-09-14  1:29 ` [ruby-core:100004] " shyouhei
2020-10-25 20:08 ` [ruby-core:100538] " marcandre-ruby-core
2020-10-29 16:06 ` [ruby-core:100646] " ko1

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).