[ruby-core:126584] [Ruby Feature#22297] Module#method_defined? should have an `include_private` argument
Issue #22297 has been reported by byroot (Jean Boussier). ---------------------------------------- Feature #22297: Module#method_defined? should have an `include_private` argument https://bugs.ruby-lang.org/issues/22297 * Author: byroot (Jean Boussier) * Status: Open ---------------------------------------- Active Record generate methods during boot time based on the database schema. However before defining every one of these methods it first check if it already exists, regardless of visibility, as to avoid overwriting use defined code. As a result, the following pattern is in a few hot spots: ```ruby if method_defined?(name) || private_method_defined?(name) # ... end ``` On our Rails monolith, `private_method_defined?` account for 1% of boot time, that's not massive, but it could very easily be eliminated if there was a way to check for a method existence regardless of its visibility. ### Proposal Currently `Module#method_defined?` signature is: ```ruby method_defined?(symbol, inherit=true) ``` I think we could make it: ```ruby method_defined?(symbol, inherit=true, include_all=false) ``` -- https://bugs.ruby-lang.org/
Issue #22297 has been updated by matz (Yukihiro Matsumoto). Accepted. `method_defined?(name, inherit = true, include_all = false)` looks good. I considered adding a new method instead, but the only difference would be visibility, and I couldn't find a good name for it. An optional positional argument is simpler, and it is consistent with `respond_to?(name, include_all)`. The argument should be added only to `method_defined?`. `public_method_defined?`, `private_method_defined?` and `protected_method_defined?` already specify visibility by name. Matz. ---------------------------------------- Feature #22297: Module#method_defined? should have an `include_private` argument https://bugs.ruby-lang.org/issues/22297#change-118926 * Author: byroot (Jean Boussier) * Status: Open ---------------------------------------- Active Record generate methods during boot time based on the database schema. However before defining every one of these methods it first check if it already exists, regardless of visibility, as to avoid overwriting use defined code. As a result, the following pattern is in a few hot spots: ```ruby if method_defined?(name) || private_method_defined?(name) # ... end ``` On our Rails monolith, `private_method_defined?` account for 1% of boot time, that's not massive, but it could very easily be eliminated if there was a way to check for a method existence regardless of its visibility. ### Proposal Currently `Module#method_defined?` signature is: ```ruby method_defined?(symbol, inherit=true) ``` I think we could make it: ```ruby method_defined?(symbol, inherit=true, include_all=false) ``` -- https://bugs.ruby-lang.org/
Issue #22297 has been updated by matheusrich (Matheus Richard). I dislike positional boolean args (see [#17938](https://bugs.ruby-lang.org/issues/17938)). I'm not aware of the performance impact, but would it be possible to use a kwarg here? ---------------------------------------- Feature #22297: Module#method_defined? should have an `include_private` argument https://bugs.ruby-lang.org/issues/22297#change-119083 * Author: byroot (Jean Boussier) * Status: Closed ---------------------------------------- Active Record generate methods during boot time based on the database schema. However before defining every one of these methods it first check if it already exists, regardless of visibility, as to avoid overwriting use defined code. As a result, the following pattern is in a few hot spots: ```ruby if method_defined?(name) || private_method_defined?(name) # ... end ``` On our Rails monolith, `private_method_defined?` account for 1% of boot time, that's not massive, but it could very easily be eliminated if there was a way to check for a method existence regardless of its visibility. ### Proposal Currently `Module#method_defined?` signature is: ```ruby method_defined?(symbol, inherit=true) ``` I think we could make it: ```ruby method_defined?(symbol, inherit=true, include_all=false) ``` -- https://bugs.ruby-lang.org/
Issue #22297 has been updated by byroot (Jean Boussier). @matheusrich I considered a keyword argument, but went with positional for consistency. This isn't yet in a released version, so feel free to come forward with a proposal to make these keyword arguments before the final 4.1 release. As for performance, yes, keyword arguments have some extra cost when parsed from C, but in this case we can probably move the argument parsing to Ruby, so probably negligible. ---------------------------------------- Feature #22297: Module#method_defined? should have an `include_private` argument https://bugs.ruby-lang.org/issues/22297#change-119087 * Author: byroot (Jean Boussier) * Status: Closed ---------------------------------------- Active Record generate methods during boot time based on the database schema. However before defining every one of these methods it first check if it already exists, regardless of visibility, as to avoid overwriting use defined code. As a result, the following pattern is in a few hot spots: ```ruby if method_defined?(name) || private_method_defined?(name) # ... end ``` On our Rails monolith, `private_method_defined?` account for 1% of boot time, that's not massive, but it could very easily be eliminated if there was a way to check for a method existence regardless of its visibility. ### Proposal Currently `Module#method_defined?` signature is: ```ruby method_defined?(symbol, inherit=true) ``` I think we could make it: ```ruby method_defined?(symbol, inherit=true, include_all=false) ``` -- https://bugs.ruby-lang.org/
participants (3)
-
byroot (Jean Boussier) -
matheusrich (Matheus Richard) -
matz (Yukihiro Matsumoto)