ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:105708] [Ruby master Bug#18258] Ractor.shareable? can be slow and mutates internal object flags.
@ 2021-10-21  3:19 ioquatix (Samuel Williams)
  2021-10-21  8:52 ` [ruby-core:105719] " Eregon (Benoit Daloze)
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-10-21  3:19 UTC (permalink / raw)
  To: ruby-core

Issue #18258 has been reported by ioquatix (Samuel Williams).

----------------------------------------
Bug #18258: Ractor.shareable? can be slow and mutates internal object flags.
https://bugs.ruby-lang.org/issues/18258

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ko1 (Koichi Sasada)
* ruby -v: 3.0.2
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
On my computer, even with a relatively small object graph,`Ractor.shareable?` can be quite slow (around 1-2ms). The following example creates an object graph with ~40k objects as an example, and on my computer takes around 20ms to execute `Ractor.shareable?`. Because the object cannot be marked as `RB_FL_SHAREABLE` because it contains mutable state, every time we check `Ractor.shareable?` it will perform the same object traversal which is the slow path.

``` ruby
require 'benchmark'

class Borked
	def freeze
	end
end

class Nested
	def initialize(count, top = true)
		if count > 0
			@nested = count.times.map{Nested.new(count - 1, false).freeze}.freeze
		end
		
		if top
			@borked = Borked.new
		end
	end
	
	attr :nested
	attr :borked
end

def test(n)
	puts "Creating nested object of size N=#{n}"
	nested = Nested.new(n).freeze
	shareable = false
	
	result = Benchmark.measure do
		shareable = Ractor.shareable?(nested)
	end

	pp result: result, shareable: shareable
end

test(8)
```

I propose we change `Ractor.shareable?` to only check `RB_FL_SHAREABLE` which gives (1) predictable and fast performance in every case and (2) avoids mutating internal object flags when performing what looks like a read-only operation.

I respect that one way of looking at `Ractor.shareable?` is as a cache for object state. But this kind of cache can lead to unpredictable performance.

As a result, something like `String#freeze` would not create objects that can be shared with Ractor. However, I believe we can mitigate this by tweaking `String#freeze` to also set `RB_FL_SHAREABLE` if possible. I believe we should apply this to more objects. It will lead to more predictable performance for Ruby.

Since there are few real-world examples of Ractor, it's hard to find real world example of the problem. However, I believe such an issue will prevent Ractor usage as even relatively small object graphs (~1000 objects) can cause 1-2ms of latency, and this particular operation does not release the GVL either which means it stalls the entire VM.

This issue came from discussion regarding https://bugs.ruby-lang.org/issues/18035 where we are considering using `RB_FL_SHAREABLE` as a flag for immutability. By fixing this issue, we make it easier to implement model for immutability because we don't need to introduce new flags and can instead reuse existing flags.



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

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

* [ruby-core:105719] [Ruby master Bug#18258] Ractor.shareable? can be slow and mutates internal object flags.
  2021-10-21  3:19 [ruby-core:105708] [Ruby master Bug#18258] Ractor.shareable? can be slow and mutates internal object flags ioquatix (Samuel Williams)
@ 2021-10-21  8:52 ` Eregon (Benoit Daloze)
  2021-10-21  8:58 ` [ruby-core:105720] " ioquatix (Samuel Williams)
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-10-21  8:52 UTC (permalink / raw)
  To: ruby-core

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


Yeah that's an alternative design.

Currently `Ractor.shareable?` semantics are "is it already shareable as in conceptually or not?".
(And the flag is just a cache for the "yes" case)
And it's not "is it already shareable as marked with Ractor.make_shareable or all instances of that class are shareable, or they are leaf frozen objects".

Note that String#freeze could only set the shareable flag if the String has no ivar, otherwise it's incorrect.

I'm not against that design, but it might be compatibility issue.

Anyway, I think no program should check `Ractor.shareable?`, probably only Ractor internals should check that, and if not shareable then raise an exception and therefore performance before that is not that big a deal.
So maybe one solution is removing `Ractor.shareable?`, and also in C-API, they seem only useful for Ractor internals, and maybe for debugging (maybe they could be only defined with `ruby -d`?).

----------------------------------------
Bug #18258: Ractor.shareable? can be slow and mutates internal object flags.
https://bugs.ruby-lang.org/issues/18258#change-94219

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ko1 (Koichi Sasada)
* ruby -v: 3.0.2
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
On my computer, even with a relatively small object graph,`Ractor.shareable?` can be quite slow (around 1-2ms). The following example creates an object graph with ~40k objects as an example, and on my computer takes around 20ms to execute `Ractor.shareable?`. Because the object cannot be marked as `RB_FL_SHAREABLE` because it contains mutable state, every time we check `Ractor.shareable?` it will perform the same object traversal which is the slow path.

``` ruby
require 'benchmark'

class Borked
	def freeze
	end
end

class Nested
	def initialize(count, top = true)
		if count > 0
			@nested = count.times.map{Nested.new(count - 1, false).freeze}.freeze
		end
		
		if top
			@borked = Borked.new
		end
	end
	
	attr :nested
	attr :borked
end

def test(n)
	puts "Creating nested object of size N=#{n}"
	nested = Nested.new(n).freeze
	shareable = false
	
	result = Benchmark.measure do
		shareable = Ractor.shareable?(nested)
	end

	pp result: result, shareable: shareable
end

test(8)
```

I propose we change `Ractor.shareable?` to only check `RB_FL_SHAREABLE` which gives (1) predictable and fast performance in every case and (2) avoids mutating internal object flags when performing what looks like a read-only operation.

I respect that one way of looking at `Ractor.shareable?` is as a cache for object state. But this kind of cache can lead to unpredictable performance.

As a result, something like `String#freeze` would not create objects that can be shared with Ractor. However, I believe we can mitigate this by tweaking `String#freeze` to also set `RB_FL_SHAREABLE` if possible. I believe we should apply this to more objects. It will lead to more predictable performance for Ruby.

Since there are few real-world examples of Ractor, it's hard to find real world example of the problem. However, I believe such an issue will prevent Ractor usage as even relatively small object graphs (~1000 objects) can cause 1-2ms of latency, and this particular operation does not release the GVL either which means it stalls the entire VM.

This issue came from discussion regarding https://bugs.ruby-lang.org/issues/18035 where we are considering using `RB_FL_SHAREABLE` as a flag for immutability. By fixing this issue, we make it easier to implement model for immutability because we don't need to introduce new flags and can instead reuse existing flags.



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

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

* [ruby-core:105720] [Ruby master Bug#18258] Ractor.shareable? can be slow and mutates internal object flags.
  2021-10-21  3:19 [ruby-core:105708] [Ruby master Bug#18258] Ractor.shareable? can be slow and mutates internal object flags ioquatix (Samuel Williams)
  2021-10-21  8:52 ` [ruby-core:105719] " Eregon (Benoit Daloze)
@ 2021-10-21  8:58 ` ioquatix (Samuel Williams)
  2021-10-21  8:58 ` [ruby-core:105721] " Eregon (Benoit Daloze)
  2023-05-14  4:44 ` [ruby-core:113474] [Ruby master Feature#18258] " jeremyevans0 (Jeremy Evans) via ruby-core
  3 siblings, 0 replies; 5+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-10-21  8:58 UTC (permalink / raw)
  To: ruby-core

Issue #18258 has been updated by ioquatix (Samuel Williams).


I feel from a predictability POV, it would definitely be advantageous to set `RB_FL_SHAREABLE` in every case it's known ahead of time to avoid unexpected overheads. Even if `Ractor.shareable?` is not public interface, it's still used internally in many many places...

----------------------------------------
Bug #18258: Ractor.shareable? can be slow and mutates internal object flags.
https://bugs.ruby-lang.org/issues/18258#change-94220

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ko1 (Koichi Sasada)
* ruby -v: 3.0.2
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
On my computer, even with a relatively small object graph,`Ractor.shareable?` can be quite slow (around 1-2ms). The following example creates an object graph with ~40k objects as an example, and on my computer takes around 20ms to execute `Ractor.shareable?`. Because the object cannot be marked as `RB_FL_SHAREABLE` because it contains mutable state, every time we check `Ractor.shareable?` it will perform the same object traversal which is the slow path.

``` ruby
require 'benchmark'

class Borked
	def freeze
	end
end

class Nested
	def initialize(count, top = true)
		if count > 0
			@nested = count.times.map{Nested.new(count - 1, false).freeze}.freeze
		end
		
		if top
			@borked = Borked.new
		end
	end
	
	attr :nested
	attr :borked
end

def test(n)
	puts "Creating nested object of size N=#{n}"
	nested = Nested.new(n).freeze
	shareable = false
	
	result = Benchmark.measure do
		shareable = Ractor.shareable?(nested)
	end

	pp result: result, shareable: shareable
end

test(8)
```

I propose we change `Ractor.shareable?` to only check `RB_FL_SHAREABLE` which gives (1) predictable and fast performance in every case and (2) avoids mutating internal object flags when performing what looks like a read-only operation.

I respect that one way of looking at `Ractor.shareable?` is as a cache for object state. But this kind of cache can lead to unpredictable performance.

As a result, something like `String#freeze` would not create objects that can be shared with Ractor. However, I believe we can mitigate this by tweaking `String#freeze` to also set `RB_FL_SHAREABLE` if possible. I believe we should apply this to more objects. It will lead to more predictable performance for Ruby.

Since there are few real-world examples of Ractor, it's hard to find real world example of the problem. However, I believe such an issue will prevent Ractor usage as even relatively small object graphs (~1000 objects) can cause 1-2ms of latency, and this particular operation does not release the GVL either which means it stalls the entire VM.

This issue came from discussion regarding https://bugs.ruby-lang.org/issues/18035 where we are considering using `RB_FL_SHAREABLE` as a flag for immutability. By fixing this issue, we make it easier to implement model for immutability because we don't need to introduce new flags and can instead reuse existing flags.



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

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

* [ruby-core:105721] [Ruby master Bug#18258] Ractor.shareable? can be slow and mutates internal object flags.
  2021-10-21  3:19 [ruby-core:105708] [Ruby master Bug#18258] Ractor.shareable? can be slow and mutates internal object flags ioquatix (Samuel Williams)
  2021-10-21  8:52 ` [ruby-core:105719] " Eregon (Benoit Daloze)
  2021-10-21  8:58 ` [ruby-core:105720] " ioquatix (Samuel Williams)
@ 2021-10-21  8:58 ` Eregon (Benoit Daloze)
  2023-05-14  4:44 ` [ruby-core:113474] [Ruby master Feature#18258] " jeremyevans0 (Jeremy Evans) via ruby-core
  3 siblings, 0 replies; 5+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-10-21  8:58 UTC (permalink / raw)
  To: ruby-core

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


I think for the semantic model it could be easier with the proposed semantics in this issue: only always-shareable objects are shareable without `Ractor.make_shareable`/`freeze` for leaf objects with no ivars/`deep_freeze/Immutable`.
Right now it's probably confusing for users that some objects are magically `shareable?` if there were enough `.freeze`.
OTOH, it seems impossible that `shareable?` returns true except if the user already froze the instance and there are no other non-frozen objects referred in it.

Cons: it would require using `Ractor.make_shareable` even more, instead of being able to just call enough `.freeze`.
Also once we have Immutable, those objects should of course be shareable, yet `Ractor.make_shareable` wouldn't be called on them.

----------------------------------------
Bug #18258: Ractor.shareable? can be slow and mutates internal object flags.
https://bugs.ruby-lang.org/issues/18258#change-94221

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ko1 (Koichi Sasada)
* ruby -v: 3.0.2
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
On my computer, even with a relatively small object graph,`Ractor.shareable?` can be quite slow (around 1-2ms). The following example creates an object graph with ~40k objects as an example, and on my computer takes around 20ms to execute `Ractor.shareable?`. Because the object cannot be marked as `RB_FL_SHAREABLE` because it contains mutable state, every time we check `Ractor.shareable?` it will perform the same object traversal which is the slow path.

``` ruby
require 'benchmark'

class Borked
	def freeze
	end
end

class Nested
	def initialize(count, top = true)
		if count > 0
			@nested = count.times.map{Nested.new(count - 1, false).freeze}.freeze
		end
		
		if top
			@borked = Borked.new
		end
	end
	
	attr :nested
	attr :borked
end

def test(n)
	puts "Creating nested object of size N=#{n}"
	nested = Nested.new(n).freeze
	shareable = false
	
	result = Benchmark.measure do
		shareable = Ractor.shareable?(nested)
	end

	pp result: result, shareable: shareable
end

test(8)
```

I propose we change `Ractor.shareable?` to only check `RB_FL_SHAREABLE` which gives (1) predictable and fast performance in every case and (2) avoids mutating internal object flags when performing what looks like a read-only operation.

I respect that one way of looking at `Ractor.shareable?` is as a cache for object state. But this kind of cache can lead to unpredictable performance.

As a result, something like `String#freeze` would not create objects that can be shared with Ractor. However, I believe we can mitigate this by tweaking `String#freeze` to also set `RB_FL_SHAREABLE` if possible. I believe we should apply this to more objects. It will lead to more predictable performance for Ruby.

Since there are few real-world examples of Ractor, it's hard to find real world example of the problem. However, I believe such an issue will prevent Ractor usage as even relatively small object graphs (~1000 objects) can cause 1-2ms of latency, and this particular operation does not release the GVL either which means it stalls the entire VM.

This issue came from discussion regarding https://bugs.ruby-lang.org/issues/18035 where we are considering using `RB_FL_SHAREABLE` as a flag for immutability. By fixing this issue, we make it easier to implement model for immutability because we don't need to introduce new flags and can instead reuse existing flags.



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

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

* [ruby-core:113474] [Ruby master Feature#18258] Ractor.shareable? can be slow and mutates internal object flags.
  2021-10-21  3:19 [ruby-core:105708] [Ruby master Bug#18258] Ractor.shareable? can be slow and mutates internal object flags ioquatix (Samuel Williams)
                   ` (2 preceding siblings ...)
  2021-10-21  8:58 ` [ruby-core:105721] " Eregon (Benoit Daloze)
@ 2023-05-14  4:44 ` jeremyevans0 (Jeremy Evans) via ruby-core
  3 siblings, 0 replies; 5+ messages in thread
From: jeremyevans0 (Jeremy Evans) via ruby-core @ 2023-05-14  4:44 UTC (permalink / raw)
  To: ruby-core; +Cc: jeremyevans0 (Jeremy Evans)

Issue #18258 has been updated by jeremyevans0 (Jeremy Evans).

Tracker changed from Bug to Feature
Status changed from Open to Closed
ruby -v deleted (3.0.2)
Backport deleted (2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN)

@headius and I discussed this and we agree it is not a bug, though potentially the situation could be improved.  You could avoid the object flag mutation using a temporary hash per call to shareable, but that would likely make it even slower.  It doesn't seem possible to cache that an object is not shareable without creating significant cache invalidation issues.  Having String#freeze set the shareable flag if the string has no instance variables seems like a good idea, but that is more of a feature request than a bug fix.  Changing `Ractor.shareable?` to only do a shallow check and not a full check seems like a large semantic change, not a bug fix, and would make Ractor.shareable? operate differently than Ractor.make_shareable. Maybe a new Ractor.flagged_shareable? method could be added to do a shallow check for the shareable flag, but I'm not sure how useful such a method would be.

----------------------------------------
Feature #18258: Ractor.shareable? can be slow and mutates internal object flags.
https://bugs.ruby-lang.org/issues/18258#change-103055

* Author: ioquatix (Samuel Williams)
* Status: Closed
* Priority: Normal
* Assignee: ko1 (Koichi Sasada)
----------------------------------------
On my computer, even with a relatively small object graph,`Ractor.shareable?` can be quite slow (around 1-2ms). The following example creates an object graph with ~40k objects as an example, and on my computer takes around 20ms to execute `Ractor.shareable?`. Because the object cannot be marked as `RB_FL_SHAREABLE` because it contains mutable state, every time we check `Ractor.shareable?` it will perform the same object traversal which is the slow path.

``` ruby
require 'benchmark'

class Borked
	def freeze
	end
end

class Nested
	def initialize(count, top = true)
		if count > 0
			@nested = count.times.map{Nested.new(count - 1, false).freeze}.freeze
		end
		
		if top
			@borked = Borked.new
		end
	end
	
	attr :nested
	attr :borked
end

def test(n)
	puts "Creating nested object of size N=#{n}"
	nested = Nested.new(n).freeze
	shareable = false
	
	result = Benchmark.measure do
		shareable = Ractor.shareable?(nested)
	end

	pp result: result, shareable: shareable
end

test(8)
```

I propose we change `Ractor.shareable?` to only check `RB_FL_SHAREABLE` which gives (1) predictable and fast performance in every case and (2) avoids mutating internal object flags when performing what looks like a read-only operation.

I respect that one way of looking at `Ractor.shareable?` is as a cache for object state. But this kind of cache can lead to unpredictable performance.

As a result, something like `String#freeze` would not create objects that can be shared with Ractor. However, I believe we can mitigate this by tweaking `String#freeze` to also set `RB_FL_SHAREABLE` if possible. I believe we should apply this to more objects. It will lead to more predictable performance for Ruby.

Since there are few real-world examples of Ractor, it's hard to find real world example of the problem. However, I believe such an issue will prevent Ractor usage as even relatively small object graphs (~1000 objects) can cause 1-2ms of latency, and this particular operation does not release the GVL either which means it stalls the entire VM.

This issue came from discussion regarding https://bugs.ruby-lang.org/issues/18035 where we are considering using `RB_FL_SHAREABLE` as a flag for immutability. By fixing this issue, we make it easier to implement model for immutability because we don't need to introduce new flags and can instead reuse existing flags.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

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

end of thread, other threads:[~2023-05-14  4:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21  3:19 [ruby-core:105708] [Ruby master Bug#18258] Ractor.shareable? can be slow and mutates internal object flags ioquatix (Samuel Williams)
2021-10-21  8:52 ` [ruby-core:105719] " Eregon (Benoit Daloze)
2021-10-21  8:58 ` [ruby-core:105720] " ioquatix (Samuel Williams)
2021-10-21  8:58 ` [ruby-core:105721] " Eregon (Benoit Daloze)
2023-05-14  4:44 ` [ruby-core:113474] [Ruby master Feature#18258] " jeremyevans0 (Jeremy Evans) via ruby-core

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).