Project

General

Profile

Feature #8158

lightweight structure for loaded features index

Added by Yura Sokolov almost 4 years ago. Updated about 1 month ago.

Status:
Assigned
Priority:
Normal
Target version:
-
[ruby-core:53688]

Description

Use lightweight structure for loaded_features index:

  • use hand made simple hash structure, which uses only one memory chunk,
  • do not store feature name string, only hash of it, since loaded_feature_path will recheck feature name on hash collision
  • use single linked lists instead of arrays for storing features indices.
  • store this lists inside one array, using array's indices as a reference.

While startup speedup improvement is relatively small compared current implementation,
this one does not need any Ruby Objects at all, so that there is no presure on GC.

https://github.com/ruby/ruby/pull/264.patch
https://github.com/ruby/ruby/pull/264.diff
https://github.com/ruby/ruby/pull/264

0001-load.c-reduce-memory-usage-of-loaded_features_index.patch View (5.99 KB) Yura Sokolov, 12/12/2016 10:39 AM

load.c-loaded_features_numindex.patch View - simplified version (4.99 KB) Yura Sokolov, 12/12/2016 11:24 AM

History

#2 [ruby-core:54010] Updated by Zachary Scott almost 4 years ago

  • Category set to core
  • File 264.patch added

#3 [ruby-core:57523] Updated by Nobuyoshi Nakada over 3 years ago

  • Assignee set to Nobuyoshi Nakada
  • Status changed from Open to Assigned

#4 [ruby-core:60285] Updated by Hiroshi SHIBATA about 3 years ago

  • Target version changed from 2.1.0 to 2.2.0

#5 [ruby-core:77515] Updated by Yura Sokolov 5 months ago

  • File 0001-load.c-reduce-memory-usage-of-loaded_features_index.patch added

#6 [ruby-core:77516] Updated by Yura Sokolov 5 months ago

I've changed implementation a bit:
Because https://bugs.ruby-lang.org/issues/12142 likely to be accepted,
I've used st_table with numtable instead of separate datastructure.

So patch now is shorter.
https://bugs.ruby-lang.org/attachments/download/6182/0001-load.c-reduce-memory-usage-of-loaded_features_index.patch

#7 [ruby-core:78595] Updated by Eric Wong 2 months ago

funny.falcon@gmail.com wrote:

So patch now is shorter.
https://bugs.ruby-lang.org/attachments/download/6182/0001-load.c-reduce-memory-usage-of-loaded_features_index.patch

Thanks; this got broken by trivial whitespace change in r57032

I'm not sure about the portability of initializing structs
with non-const values for some compilers. Maybe usa or
somebody with more portability experience can comment.

Also, in C Ruby; initial indent is 4 spaces, second indent is
hard tab. Not my rule and I hate it; but not much we can do...

#8 [ruby-core:78596] Updated by Usaku NAKAMURA 2 months ago

Hi,

In message " Re: [Ruby trunk Feature#8158] lightweight structure for loaded features index"
on Mon, 12 Dec 2016 06:06:13 +0000, normalperson@yhbt.net wrote:

I'm not sure about the portability of initializing structs
with non-const values for some compilers. Maybe usa or
somebody with more portability experience can comment.

It's OK, at least for VC++.

Regards,
--
U.Nakamaura usa@garbagecollect.jp

#9 [ruby-core:78598] Updated by Nobuyoshi Nakada 2 months ago

Usaku NAKAMURA wrote:

In message " Re: [Ruby trunk Feature#8158] lightweight structure for loaded features index"
on Mon, 12 Dec 2016 06:06:13 +0000, normalperson@yhbt.net wrote:

I'm not sure about the portability of initializing structs
with non-const values for some compilers. Maybe usa or
somebody with more portability experience can comment.

It's OK, at least for VC++.

Other compilers fail, IIRC, Solaris, AIX, HP-UX, or something.

#10 [ruby-core:78599] Updated by Yura Sokolov 2 months ago

I'll fix patch today.

#11 Updated by Yura Sokolov 2 months ago

  • File deleted (264.patch)

#12 Updated by Yura Sokolov 2 months ago

  • File deleted (0001-load.c-reduce-memory-usage-of-loaded_features_index.patch)

#15 [ruby-core:78604] Updated by Yura Sokolov 2 months ago

  • File load.c-loaded_features_numindex.patch added

#16 Updated by Yura Sokolov 2 months ago

  • File deleted (load.c-loaded_features_numindex.patch)

#19 [ruby-core:78780] Updated by Shyouhei Urabe 2 months ago

We looked at this issue at today's developer meeting and had positive opinions. Maybe introduced in 2.5.

#20 [ruby-core:79133] Updated by Yukihiro Matsumoto about 1 month ago

The patch seems OK now. @funny_falcon do you want to check in by yourself?

Matz.

#21 [ruby-core:79164] Updated by Yura Sokolov about 1 month ago

Matz,

I don't know English enough to clearly understand your suggestion.

Is it suggestion of commit rights?
If it is, it will be a great honor for me.
But I'm not even-tempered person, and I fear I will put a mess into repository if I will have commit rights.

If your suggestion means some-thing else, then may you formulate it in some other words so I can understand?

With regards,
Yura.

Also available in: Atom PDF