Project

General

Profile

Feature #8158

lightweight structure for loaded features index

Added by funny_falcon (Yura Sokolov) over 5 years ago. Updated 6 months ago.

Status:
Closed
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 (5.99 KB) 0001-load.c-reduce-memory-usage-of-loaded_features_index.patch funny_falcon (Yura Sokolov), 12/12/2016 10:39 AM
load.c-loaded_features_numindex.patch (4.99 KB) load.c-loaded_features_numindex.patch simplified version funny_falcon (Yura Sokolov), 12/12/2016 11:24 AM

Related issues

Related to Ruby trunk - Feature #14460: Speed up `require` and reduce memory usageClosed

History

#2 [ruby-core:54010] Updated by zzak (Zachary Scott) over 5 years ago

  • File 264.patch added
  • Category set to core

#3 [ruby-core:57523] Updated by nobu (Nobuyoshi Nakada) almost 5 years ago

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

#4 [ruby-core:60285] Updated by hsbt (Hiroshi SHIBATA) over 4 years ago

  • Target version changed from 2.1.0 to 2.2.0

#5 [ruby-core:77515] Updated by funny_falcon (Yura Sokolov) almost 2 years ago

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

#6 [ruby-core:77516] Updated by funny_falcon (Yura Sokolov) almost 2 years 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 normalperson (Eric Wong) over 1 year 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 usa (Usaku NAKAMURA) over 1 year 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 nobu (Nobuyoshi Nakada) over 1 year 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 funny_falcon (Yura Sokolov) over 1 year ago

I'll fix patch today.

#11 Updated by funny_falcon (Yura Sokolov) over 1 year ago

  • File deleted (264.patch)

#12 Updated by funny_falcon (Yura Sokolov) over 1 year ago

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

#15 [ruby-core:78604] Updated by funny_falcon (Yura Sokolov) over 1 year ago

  • File load.c-loaded_features_numindex.patch added

#16 Updated by funny_falcon (Yura Sokolov) over 1 year ago

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

#19 [ruby-core:78780] Updated by shyouhei (Shyouhei Urabe) over 1 year 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 matz (Yukihiro Matsumoto) over 1 year ago

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

Matz.

#21 [ruby-core:79164] Updated by funny_falcon (Yura Sokolov) over 1 year 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.

#22 [ruby-core:85530] Updated by sam.saffron (Sam Saffron) 6 months ago

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

#23 Updated by duerst (Martin Dürst) 6 months ago

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

#24 [ruby-core:85543] Updated by funny_falcon (Yura Sokolov) 6 months ago

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.

#25 [ruby-core:85554] Updated by tenderlovemaking (Aaron Patterson) 6 months ago

  • Status changed from Assigned to Closed

Committed in r62404. Thanks Yura!!

#26 [ruby-core:85588] Updated by normalperson (Eric Wong) 6 months ago

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.

#28 [ruby-core:85690] Updated by funny_falcon (Yura Sokolov) 6 months ago

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.

#29 [ruby-core:85691] Updated by funny_falcon (Yura Sokolov) 6 months ago

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.

#30 [ruby-core:85774] Updated by hsbt (Hiroshi SHIBATA) 6 months ago

Hi all.

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

Also available in: Atom PDF