Feature #8158: lightweight structure for loaded features index

• Status: Closed
• Assignee: nobu (Nobuyoshi Nakada)
• Target version: -

[funny_falcon (Yura Sokolov)]

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

[funny_falcon (Yura Sokolov)]

Same for ruby_2_0_0 branch

https://github.com/funny-falcon/ruby/compare/ruby_2_0_0...features_index/ruby_2_0_0.diff
https://github.com/funny-falcon/ruby/compare/ruby_2_0_0...features_index/ruby_2_0_0

[zzak (zzak _)]

• File 264.patch added
• Category set to core

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Assigned
• Assignee set to nobu (Nobuyoshi Nakada)

[hsbt (Hiroshi SHIBATA)]

• Target version changed from 2.1.0 to 2.2.0

[funny_falcon (Yura Sokolov)]

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

[funny_falcon (Yura Sokolov)]

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

[normalperson (Eric Wong)]

[e-mail] 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...

[usa (Usaku NAKAMURA)]

Hi,

In message "[ruby-core:78595] Re: [Ruby trunk Feature#8158] lightweight structure for loaded features index"
on Mon, 12 Dec 2016 06:06:13 +0000, [e-mail] 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 [e-mail]

[nobu (Nobuyoshi Nakada)]

Usaku NAKAMURA wrote:

In message "[ruby-core:78595] Re: [Ruby trunk Feature#8158] lightweight structure for loaded features index"
on Mon, 12 Dec 2016 06:06:13 +0000, [e-mail] 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.

[funny_falcon (Yura Sokolov)]

I'll fix patch today.

[funny_falcon (Yura Sokolov)]

• File deleted (264.patch)

[funny_falcon (Yura Sokolov)]

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

[funny_falcon (Yura Sokolov)]

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

[funny_falcon (Yura Sokolov)]

I've uploaded fixed patch:
https://bugs.ruby-lang.org/attachments/download/6293/0001-load.c-reduce-memory-usage-of-loaded_features_index.patch

Github branch and diff:
https://github.com/funny-falcon/ruby/tree/loaded_features_strings
https://github.com/ruby/ruby/compare/trunk...funny-falcon:loaded_features_strings.patch

[funny_falcon (Yura Sokolov)]

• File load.c-loaded_features_numindex.patch added

[funny_falcon (Yura Sokolov)]

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

[funny_falcon (Yura Sokolov)]

• File load.c-loaded_features_numindex.patch load.c-loaded_features_numindex.patch added

[funny_falcon (Yura Sokolov)]

I've tried to simplify patch: remove struct feature_str and use just pointer with len.

https://bugs.ruby-lang.org/attachments/download/6295/load.c-loaded_features_numindex.patch

https://github.com/ruby/ruby/compare/trunk...funny-falcon:loaded_features_index.patch
https://github.com/funny-falcon/ruby/tree/loaded_features_index

[shyouhei (Shyouhei Urabe)]

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

[matz (Yukihiro Matsumoto)]

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

Matz.

[funny_falcon (Yura Sokolov)]

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.

[sam.saffron (Sam Saffron)]

Hi Yura,

I think the commit rights here reduce the amount of work for the rest of the Ruby team.

This change is approved and reviewed, so you can commit it directly once you have the rights, no need to make other people do the committing.

Very keen to have this change included in Ruby.

Sam

[duerst (Martin Dürst)]

• Related to Feature #14460: Speed up `require` and reduce memory usage added

[funny_falcon (Yura Sokolov)]

Year after I think I am more stable person, so I'm going to accept commit rights with gratitude, honor and great responsibility.

But until I passed procedure, I'd be thankful if @tenderlovemaking (Aaron Patterson) will commit this one.

With regards,
Yura.

[tenderlovemaking (Aaron Patterson)]

• Status changed from Assigned to Closed

Committed in r62404. Thanks Yura!!

[normalperson (Eric Wong)]

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

I am curious, what is the significance of the 0xfea7009e
initializer passed to st_hash?

Would it be appropriate to use for the `st_hash' call in
hash.c::hash_i as well?

Thanks.

[matz (Yukihiro Matsumoto)]

@funny_falcon (Yura Sokolov)? Will you be a committer?

Matz.

[funny_falcon (Yura Sokolov)]

@matz (Yukihiro Matsumoto) , yes I will.
Excuse me for the delay. I'll try to proceed with steps from CommitterHowto this week.

With regards,
Yura.

[funny_falcon (Yura Sokolov)]

normalperson (Eric Wong) wrote:

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

I am curious, what is the significance of the 0xfea7009e
initializer passed to st_hash?

0xfea7009e - is a "feature" hex-spelled :-) Nothing special.
It could be ommitted, given features are strings without zero bytes.
I put it here just for fun.

Would it be appropriate to use for the `st_hash' call in
hash.c::hash_i as well?

I believe, there's no real needs.

Thanks.

With regards,
Yura.

[hsbt (Hiroshi SHIBATA)]

Hi all.

I did create an account of funny_falcon on svn.ruby-lang.org.