Project

General

Profile

Bug #11977

Bug with array literals caused by r53376

Added by tenderlovemaking (Aaron Patterson) almost 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Target version:
-
ruby -v:
ruby 2.4.0dev (2016-01-11 trunk 53499) [x86_64-darwin15]
[ruby-core:72807]

Description

Hi,

I'm seeing a bug with the class name of array literals, and I believe it is caused by r53376 (reverting that commit seems to fix the issue).

The bug is that array literals will have the class name "ThreadSafe::Array" when I expect them to have the class name "Array". Here is the output I'm seeing:

[aaron@TC testing (master)]$ bundle exec ruby -w -e 'p [].class'
Array
[aaron@TC testing (master)]$ bundle exec ruby -w -e 'require "rails"; p [].class'
ThreadSafe::Array
[aaron@TC testing (master)]$

I'm trying to reduce the problem, but so far I can only reproduce this issue inside a Rails application that requires the "concurrent-ruby" gem.

I've posted a test application here:

https://github.com/tenderlove/array_bug.git

I think the bug is to do with autoload handling changes in r53376. Also I've found that you must use bundler, so it could also be related to bundler's manipulation of the load path.

Thanks.


Related issues

Related to Ruby master - Bug #13120: p [].class shows ThreadSafe::Array when it expects to show ArrayClosedActions

Associated revisions

Revision 244916d4
Added by normal almost 4 years ago

resolve class name earlier and more consistently

This further avoids class name resolution issues which came
about due to relying on hash table ordering before r53376.

Pre-caching the class name when it is never used raises memory
use, but the overall gain from moving away from st still gives
us a small gain. Reverting r53376 and this patch and testing with
"valgrind -v ./ruby -rrdoc -eexit" on x86 (32-bit) shows:

before:
in use at exit: 1,662,239 bytes in 25,286 blocks
total heap usage: 49,514 allocs, 24,228 frees, 6,005,561 bytes allocated

after, with this change:
in use at exit: 1,646,529 bytes in 24,572 blocks
total heap usage: 48,891 allocs, 24,319 frees, 6,003,921 bytes allocated

  • class.c (Init_class_hierarchy): resolve name for rb_cObject ASAP
  • object.c (rb_mod_const_set): move name resolution to rb_const_set
  • variable.c (rb_const_set): do class resolution here [ruby-core:72807] [Bug #11977]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@53518 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 53518
Added by normalperson (Eric Wong) almost 4 years ago

resolve class name earlier and more consistently

This further avoids class name resolution issues which came
about due to relying on hash table ordering before r53376.

Pre-caching the class name when it is never used raises memory
use, but the overall gain from moving away from st still gives
us a small gain. Reverting r53376 and this patch and testing with
"valgrind -v ./ruby -rrdoc -eexit" on x86 (32-bit) shows:

before:
in use at exit: 1,662,239 bytes in 25,286 blocks
total heap usage: 49,514 allocs, 24,228 frees, 6,005,561 bytes allocated

after, with this change:
in use at exit: 1,646,529 bytes in 24,572 blocks
total heap usage: 48,891 allocs, 24,319 frees, 6,003,921 bytes allocated

  • class.c (Init_class_hierarchy): resolve name for rb_cObject ASAP
  • object.c (rb_mod_const_set): move name resolution to rb_const_set
  • variable.c (rb_const_set): do class resolution here [ruby-core:72807] [Bug #11977]

Revision 53518
Added by normal almost 4 years ago

resolve class name earlier and more consistently

This further avoids class name resolution issues which came
about due to relying on hash table ordering before r53376.

Pre-caching the class name when it is never used raises memory
use, but the overall gain from moving away from st still gives
us a small gain. Reverting r53376 and this patch and testing with
"valgrind -v ./ruby -rrdoc -eexit" on x86 (32-bit) shows:

before:
in use at exit: 1,662,239 bytes in 25,286 blocks
total heap usage: 49,514 allocs, 24,228 frees, 6,005,561 bytes allocated

after, with this change:
in use at exit: 1,646,529 bytes in 24,572 blocks
total heap usage: 48,891 allocs, 24,319 frees, 6,003,921 bytes allocated

  • class.c (Init_class_hierarchy): resolve name for rb_cObject ASAP
  • object.c (rb_mod_const_set): move name resolution to rb_const_set
  • variable.c (rb_const_set): do class resolution here [ruby-core:72807] [Bug #11977]

Revision 53518
Added by normal almost 4 years ago

resolve class name earlier and more consistently

This further avoids class name resolution issues which came
about due to relying on hash table ordering before r53376.

Pre-caching the class name when it is never used raises memory
use, but the overall gain from moving away from st still gives
us a small gain. Reverting r53376 and this patch and testing with
"valgrind -v ./ruby -rrdoc -eexit" on x86 (32-bit) shows:

before:
in use at exit: 1,662,239 bytes in 25,286 blocks
total heap usage: 49,514 allocs, 24,228 frees, 6,005,561 bytes allocated

after, with this change:
in use at exit: 1,646,529 bytes in 24,572 blocks
total heap usage: 48,891 allocs, 24,319 frees, 6,003,921 bytes allocated

  • class.c (Init_class_hierarchy): resolve name for rb_cObject ASAP
  • object.c (rb_mod_const_set): move name resolution to rb_const_set
  • variable.c (rb_const_set): do class resolution here [ruby-core:72807] [Bug #11977]

Revision 53518
Added by normal almost 4 years ago

resolve class name earlier and more consistently

This further avoids class name resolution issues which came
about due to relying on hash table ordering before r53376.

Pre-caching the class name when it is never used raises memory
use, but the overall gain from moving away from st still gives
us a small gain. Reverting r53376 and this patch and testing with
"valgrind -v ./ruby -rrdoc -eexit" on x86 (32-bit) shows:

before:
in use at exit: 1,662,239 bytes in 25,286 blocks
total heap usage: 49,514 allocs, 24,228 frees, 6,005,561 bytes allocated

after, with this change:
in use at exit: 1,646,529 bytes in 24,572 blocks
total heap usage: 48,891 allocs, 24,319 frees, 6,003,921 bytes allocated

  • class.c (Init_class_hierarchy): resolve name for rb_cObject ASAP
  • object.c (rb_mod_const_set): move name resolution to rb_const_set
  • variable.c (rb_const_set): do class resolution here [ruby-core:72807] [Bug #11977]

History

Updated by tenderlovemaking (Aaron Patterson) almost 4 years ago

Eric Wong wrote:

I can't seem to reproduce the problem with array_bug.git on my end.
Can you try this?

http://80x24.org/spew/20160112021410.27364-1-e%4080x24.org/raw

Thanks for the patch. That indeed fixes it for me. Can you apply it?

I guess this is due to the hash table change? I can't say I fully understand why this fixes it, besides pre-populating the cache.

#3

Updated by Anonymous almost 4 years ago

  • Status changed from Open to Closed

Applied in changeset r53518.


resolve class name earlier and more consistently

This further avoids class name resolution issues which came
about due to relying on hash table ordering before r53376.

Pre-caching the class name when it is never used raises memory
use, but the overall gain from moving away from st still gives
us a small gain. Reverting r53376 and this patch and testing with
"valgrind -v ./ruby -rrdoc -eexit" on x86 (32-bit) shows:

before:
in use at exit: 1,662,239 bytes in 25,286 blocks
total heap usage: 49,514 allocs, 24,228 frees, 6,005,561 bytes allocated

after, with this change:
in use at exit: 1,646,529 bytes in 24,572 blocks
total heap usage: 48,891 allocs, 24,319 frees, 6,003,921 bytes allocated

  • class.c (Init_class_hierarchy): resolve name for rb_cObject ASAP
  • object.c (rb_mod_const_set): move name resolution to rb_const_set
  • variable.c (rb_const_set): do class resolution here [ruby-core:72807] [Bug #11977]

Updated by normalperson (Eric Wong) almost 4 years ago

tenderlove@ruby-lang.org wrote:

Thanks for the patch. That indeed fixes it for me. Can you apply it?

Thanks for the confirmation, r53518

I guess this is due to the hash table change? I can't say I fully understand
why this fixes it, besides pre-populating the cache.

Yes, it used to be order-dependent.

Populating the cache early means we resolve the name before other entries
have a chance to mess with the ordering. So when an entry is inserted
relative to another still affects how its name is resolved.

Unsubscribe: ruby-core-request@ruby-lang.org?subject=unsubscribe
http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core

Updated by tenderlovemaking (Aaron Patterson) almost 4 years ago

Eric Wong wrote:

tenderlove@ruby-lang.org wrote:

I guess this is due to the hash table change? I can't say I fully understand
why this fixes it, besides pre-populating the cache.

Yes, it used to be order-dependent.

Populating the cache early means we resolve the name before other entries
have a chance to mess with the ordering. So when an entry is inserted
relative to another still affects how its name is resolved.

Ah, interesting. Thank you for the explanation and the fix. I appreciate it!

#6

Updated by shyouhei (Shyouhei Urabe) almost 3 years ago

  • Related to Bug #13120: p [].class shows ThreadSafe::Array when it expects to show Array added

Also available in: Atom PDF