Project

General

Profile

Actions

Feature #19377

closed

Rename Fiber#storage to Fiber.storage

Added by zverok (Victor Shepelev) about 1 year ago. Updated about 1 year ago.

Status:
Rejected
Assignee:
-
Target version:
-
[ruby-core:112041]

Description

Justification:

  • #storage, which pretends to be an instance method, is always available only on Fiber.current, which is problematic to document, and needs special handling when storage is called for non-current Fiber;
  • with class method + docs "storage of the current fiber" it all will be self-evident;
  • Most of the time, when we declare methods that are available only in the current {something}, they are class methods. (like storage's itself interface of Fiber::[] and Fiber::[]=, or Module.used_modules, which is modules and refinements of the current context, but it is not self.used_modules, for example)
  • Code like
Fiber.current.storage = {foo: 'bar'}
Fiber[:foo]

...looks like it is two unrelated things (storage of the current fiber vs some "global" key getter/setter?)

I don't see much discussion of this in #19078. Matz in #19078#note-22, while accepting the interface, describes it as homogenous:

(1) fiber[key]/fiber[key]=val - accepted.
(2) fiber.storage => hash - accepted
(3) fiber.storage=hash - should be experimental
...

So I believe it should be either Fiber.current[] and Fiber.current.storage, or Fiber[], and Fiber.storage. The latter is preferable to underline it is only one, related to the current fiber, interface, not "every fiber instance has one (but actually you can use only current's)

Actions #1

Updated by zverok (Victor Shepelev) about 1 year ago

  • Description updated (diff)

Updated by ioquatix (Samuel Williams) about 1 year ago

It's useful for debugging to access the storage of a different fiber.

Updated by zverok (Victor Shepelev) about 1 year ago

@ioquatix (Samuel Williams),

It's useful for debugging to access the storage of a different fiber.

Can you please expand on this?.. How it can be done for debugging?..

Updated by Eregon (Benoit Daloze) about 1 year ago

Currently Fiber#storage can only be called for the current Fiber (otherwise ArgumentError).
But it may evolve (e.g. allow to be called on different Fiber but Fiber of same thread), this is why it's Fiber#storage and not Fiber.storage.
Additionally, Fiber#storage and Fiber#storage= are expected to be much rarer to use.
IMHO it would feel weird to have them as class methods on Fiber.
For instance Fiber#storage shouldn't be used by regular users, only for debugging or for copying storage from one Fiber to another (e.g. useful for thread pools).

Updated by zverok (Victor Shepelev) about 1 year ago

@Eregon (Benoit Daloze) My argument was not about #storage taken alone, but about #storage vs ::[] inconsistency. The inconsistency can be fixed either way (e.g. if "it may evolve", then why don't we do Fiber.current['name'] from the very beginning?)

Additionally, Fiber#storage and Fiber#storage= are expected to be much rarer to use.

Right, but methods like this are frequently used when learning the language and investigating/debugging "how things are", so again, inconsistency strikes here.

IMHO it would feel weird to have them as class methods on Fiber.

Does Fiber[foo] not feel weird? (For me it actually does, seems like something "global", not local to the current fiber at all)

Updated by Eregon (Benoit Daloze) about 1 year ago

zverok (Victor Shepelev) wrote in #note-5:

Does Fiber[foo] not feel weird? (For me it actually does, seems like something "global", not local to the current fiber at all)

For me no, it's actually a nice way to communicate you can only access those Fiber storage variables for the current Fiber.

I wish we would also have Thread[:foo] BTW.
Unfortunately for true fiber-local variables, Fiber[:foo] is already taken by Fiber storage variables.
And we can't move Fiber storage variables to Fiber.current[:foo] because that's already true fiber-local variables.
I mentioned this in the original ticket, I wish we had longer more explicit names for Fiber storage variables to avoid this confusion potential.

So we can't move Fiber[] to Fiber#[] (already taken).
And while we could have Fiber.storage{,=}, it's rather limiting to evolve it, and IMO as confusing or worse if we have both Fiber.storage and Fiber#storage.

Updated by zverok (Victor Shepelev) about 1 year ago

  • Status changed from Open to Rejected

For me no, it's actually a nice way to communicate you can only access those Fiber storage variables for the current Fiber.

Hmm, I see. So you are saying that this difference of protocols is not a "bug" (conceptually), but a feature, underlining that:

  • [] will always be only current Fiber's variables direct access and it is "layman user's API";
  • while Fiber#storage is more of a "technical" API, which might in future be extended, but not recommended for everyday code.

OK, makes sense probably.

Updated by Eregon (Benoit Daloze) about 1 year ago

Yes, exactly.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0