ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: merch-redmine@jeremyevans.net
To: ruby-core@ruby-lang.org
Subject: [ruby-core:99459] [Ruby master Feature#17055] Allow suppressing uninitialized instance variable and method redefined verbose mode warnings
Date: Mon, 03 Aug 2020 17:01:26 +0000 (UTC)	[thread overview]
Message-ID: <redmine.journal-86916.20200803170126.1604@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-17055.20200728220329.1604@ruby-lang.org

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


tenderlovemaking (Aaron Patterson) wrote in #note-14:
> jeremyevans0 (Jeremy Evans) wrote in #note-13:
> > As you'll see by the benchmark, the reason the performance difference is much different than you would expect is that Model loads from the database in Sequel run through `Sequel::Model.call` (class method, not instance method), and all of the plugins that use instance variables must override the method to set the instance variables.  Because it is a class method and not an instance method, `instance_variable_set` must be used.  The overhead of all of those additional method calls (`super` and `instance_variable_set`) is what causes the dramatic difference in performance.
> 
> Could the design be improved such that `instance_variable_set` isn't required?  Using `instance_variable_set` means that inline caches will not be used (in MRI anyway), so I'm not sure it should be used in code that requires high performance.  I suppose we could also work on improving the performance of `instance_variable_set`, but I'm not sure usage is _that_ common.

It certainly could, but it would require more work, and would result in slower performance in the the no-plugin case.  We could add a private instance method can call that, and the private instance method could initialize all the instance variables to nil the usual way.  That would make things somewhat faster.  You still have all the super calls to slow things down, though.  If you want me to work on a benchmark for that, please let me know, but it's a sure bet that even that approach would result in a slowdown significant enough that I wouldn't want to switch to it.

----------------------------------------
Feature #17055: Allow suppressing uninitialized instance variable and method redefined verbose mode warnings
https://bugs.ruby-lang.org/issues/17055#change-86916

* Author: jeremyevans0 (Jeremy Evans)
* Status: Open
* Priority: Normal
----------------------------------------
These two verbose mode warnings are both fairly common and have good reasons why you would not want to warn about them in specific cases.  Not initializing instance variables to nil can be much better for performance, and redefining methods without removing the method first is the only safe approach in multi-threaded code.

There are reasons that you may want to issue verbose warnings by default in these cases.  For uninitialized instance variables, it helps catch typos. For method redefinition, it could alert you that a method already exists when you didn't expect it to, such as when a file is loaded multiple times when it should only be loaded once.

I propose we keep the default behavior the same, but offer the ability to opt-out of these warnings by defining methods.  For uninitialized instance variables in verbose mode, I propose we call `expected_uninitialized_instance_variable?(iv)` on the object.  If this method doesn't exist or returns false/nil, we issue the warning.  If the method exists and returns true, we suppress the warning.  Similarly, for redefined methods, we call `expected_redefined_method?(method_name)` on the class or module.  If the method doesn't exist or returns false/nil, we issue the warning.  If the method exists and returns true, we suppress the warning.

This approach allows high performance code (uninitialized instance variables) and safe code (redefining methods without removing) to work without verbose mode warnings.

I have implemented this support in a pull request: https://github.com/ruby/ruby/pull/3371

---Files--------------------------------
t.rb (5.59 KB)


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

  parent reply	other threads:[~2020-08-03 17:01 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-28 22:03 [ruby-core:99375] [Ruby master Feature#17055] Allow suppressing uninitialized instance variable and method redefined verbose mode warnings merch-redmine
2020-07-29  2:26 ` [ruby-core:99386] " shyouhei
2020-07-29  6:54 ` [ruby-core:99387] " kamipo
2020-07-29  9:17 ` [ruby-core:99388] " jean.boussier
2020-07-29 20:48 ` [ruby-core:99395] " eregontp
2020-07-29 20:55 ` [ruby-core:99396] " eregontp
2020-07-29 21:37 ` [ruby-core:99397] " merch-redmine
2020-07-30 18:04 ` [ruby-core:99400] " headius
2020-07-30 18:16 ` [ruby-core:99401] " headius
2020-08-01 14:36 ` [ruby-core:99439] " eregontp
2020-08-01 14:54 ` [ruby-core:99440] " eregontp
2020-08-02 15:38 ` [ruby-core:99445] " merch-redmine
2020-08-03 11:22 ` [ruby-core:99453] " eregontp
2020-08-03 16:30 ` [ruby-core:99457] " merch-redmine
2020-08-03 16:48 ` [ruby-core:99458] " tenderlove
2020-08-03 17:01 ` merch-redmine [this message]
2020-08-03 18:31 ` [ruby-core:99460] " tenderlove
2020-08-04  2:59 ` [ruby-core:99472] " merch-redmine
2020-08-15 10:54 ` [ruby-core:99595] " eregontp
2020-08-15 12:23 ` [ruby-core:99596] " eregontp
2020-08-15 15:42 ` [ruby-core:99597] " merch-redmine
2020-09-01 15:56 ` [ruby-core:99819] " merch-redmine
2020-09-02 14:57 ` [ruby-core:99844] " eregontp
2020-09-02 15:35 ` [ruby-core:99845] " merch-redmine
2020-12-02 21:25 ` [ruby-core:101207] " merch-redmine
2020-12-03 22:44   ` [ruby-core:101231] " Austin Ziegler
2020-12-03 23:04     ` [ruby-core:101233] " Jeremy Evans
2020-12-04 17:34       ` [ruby-core:101244] " Austin Ziegler
2020-12-10  4:35 ` [ruby-core:101353] " matz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-86916.20200803170126.1604@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).