From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS4713 221.184.0.0/13 X-Spam-Status: No, score=-2.8 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FORGED_GMAIL_RCVD,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, SPF_PASS shortcircuit=no autolearn=no autolearn_force=no version=3.4.2 Received: from neon.ruby-lang.org (neon.ruby-lang.org [221.186.184.75]) by dcvr.yhbt.net (Postfix) with ESMTP id E78261F45F for ; Wed, 8 May 2019 11:30:47 +0000 (UTC) Received: from neon.ruby-lang.org (localhost [IPv6:::1]) by neon.ruby-lang.org (Postfix) with ESMTP id B8919120A7E; Wed, 8 May 2019 20:30:41 +0900 (JST) Received: from o1678916x28.outbound-mail.sendgrid.net (o1678916x28.outbound-mail.sendgrid.net [167.89.16.28]) by neon.ruby-lang.org (Postfix) with ESMTPS id B1ED9120A85 for ; Wed, 8 May 2019 20:30:38 +0900 (JST) Received: by filter0163p3mdw1.sendgrid.net with SMTP id filter0163p3mdw1-32580-5CD2BDDA-58 2019-05-08 11:30:34.952464358 +0000 UTC m=+48527.393913687 Received: from herokuapp.com (unknown [3.88.90.180]) by ismtpd0036p1iad1.sendgrid.net (SG) with ESMTP id B_e12LCfQV-3rVVkJStKpw for ; Wed, 08 May 2019 11:30:34.914 +0000 (UTC) Date: Wed, 08 May 2019 11:30:35 +0000 (UTC) From: jean.boussier@gmail.com Message-ID: References: Mime-Version: 1.0 X-Redmine-MailingListIntegration-Message-Ids: 68080 X-Redmine-Project: ruby-trunk X-Redmine-Issue-Id: 15836 X-Redmine-Issue-Author: byroot X-Redmine-Sender: byroot X-Mailer: Redmine X-Redmine-Host: bugs.ruby-lang.org X-Redmine-Site: Ruby Issue Tracking System X-Auto-Response-Suppress: All Auto-Submitted: auto-generated X-SG-EID: =?us-ascii?Q?AchqQMoUBMcQgz7gop0XiYUiatGIY7E61JGsTL4Fvjd76PDMpjrPziyucRZxCx?= =?us-ascii?Q?f7loCChcLEKwzRkEeoXCtsutGSWXqWPis6RacLs?= =?us-ascii?Q?aiF+NBWv+Jl1jGweAHwRvevvv7cQnROeqIZTr36?= =?us-ascii?Q?J1jVtGtOy1VTgdiXsbdOFlu4simkL5k19+Ducea?= =?us-ascii?Q?UBQjXIFJvv3+qeEUWtwmQEKlK4j2EmzWaMQ=3D=3D?= To: ruby-core@ruby-lang.org X-ML-Name: ruby-core X-Mail-Count: 92597 Subject: [ruby-core:92597] [Ruby trunk Feature#15836] [Proposal] Make Module#name and Symbol#to_s return their internal fstrings X-BeenThere: ruby-core@ruby-lang.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: Ruby developers List-Id: Ruby developers List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ruby-core-bounces@ruby-lang.org Sender: "ruby-core" Issue #15836 has been updated by byroot (Jean Boussier). @mame re benchmark So I decided to run this against redmine boot, using this branch: https://github.com/redmine/redmine/compare/master...byroot:boot-benchmark Eager loading is enabled so that the entire codebase is loaded, and it uses https://github.com/SamSaffron/memory_profiler to measure allocations and retentions. Full benchmark output: https://gist.github.com/byroot/845a5877c1cde91c50b43be446dfb20f Baseline (official 2.6.3): ``` Total allocated: 121.11 MB (1234362 objects) Total retained: 24.86 MB (200539 objects) allocated memory by class ----------------------------------- 63.36 MB String allocated objects by class ----------------------------------- 980623 String ``` With the patch (official 2.6.3 + this patch): ``` Total allocated: 120.01 MB (1206699 objects) Total retained: 24.82 MB (199397 objects) allocated memory by class ----------------------------------- 62.25 MB String allocated objects by class ----------------------------------- 952953 String ``` Diff: ``` -27 663 allocations (-2.24%) -1.10MB allocations (-0.9%) -1 142 retentions -0.4MB retentions ``` IMHO that is significant, especially for a small sized application like Redmine. However I can't say wether it outweigh the backward compatibility concern or not. ### Backward compatibility One thing to note is that I had to patch https://github.com/rails/rails/blob/28aca474d48b6acdbe8c7861d9347e27c65fafd9/activesupport/lib/active_support/ordered_options.rb#L43 because it was mutating the result of `Symbol#to_s`. Also running the Redmine test suite shows a couple breakage in the i18n gem. IMHO these are fairly simple to fix, but I would totally understand if that was considered as a no-go. ### Typical code benefiting from this change - Rails autoloader and Zeitwerk would both benefit from the `Module#name` change as they both keep references to class names as hash keys - Various parts of Rails would benefit as well since they use the class names extensively to derive other class names, as well as symbols `belongs_to :post`. - `def method_missing` very often call `name.to_s` to match the method name, hence would benefit from the `Symbol#to_s` as well. - Serialization of symbols into various formats, e.g. `{foo: 42}.to_json`. That pattern is fairly common IMO. ---------------------------------------- Feature #15836: [Proposal] Make Module#name and Symbol#to_s return their internal fstrings https://bugs.ruby-lang.org/issues/15836#change-77957 * Author: byroot (Jean Boussier) * Status: Feedback * Priority: Normal * Assignee: * Target version: ---------------------------------------- # Why ? In many codebases, especially Rails apps, these two methods are the source of quite a lot of object allocations. `Module#name` is often accessed for various introspection features, autoloading etc. `Symbol#to_s` is access a lot by HashWithIndifferentAccess other various APIs accepting both symbols and strings. Returning fstrings for both of these methods could significantly reduce allocations, as well as sligthly reduce retention as it would reduce some duplications. Also, more and more Ruby APIs are now returning fstrings. `frozen_string_literal`AFAIK should become the default some day, string used as hash keys are now automatically interned as well. ### Backward compatibilty Of course this is not fully backward compatible, it's inevitable that some code in the wild is mutating the strings returned by these methods, but I do believe it's a rare occurence, and easy to fix. ### Implementation I implemented it here: https://github.com/ruby/ruby/pull/2175 -- https://bugs.ruby-lang.org/