Skip to content

Add ruby_upb_alloc using xrealloc/xfree so Ruby GC is aware of allocated memory for Arenas.#9586

Merged
zhangskz merged 5 commits into
protocolbuffers:3.20.xfrom
zhangskz:master
Mar 8, 2022
Merged

Add ruby_upb_alloc using xrealloc/xfree so Ruby GC is aware of allocated memory for Arenas.#9586
zhangskz merged 5 commits into
protocolbuffers:3.20.xfrom
zhangskz:master

Conversation

@zhangskz

@zhangskz zhangskz commented Mar 4, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Add ruby-specific upb_alloc using xrealloc/xfree for use in Arena_alloc so Ruby GC is aware of allocated memory. Fixes #9546.

Since ruby is now aware of memory allocated for ruby arena object, RB_GC_GUARD must be used to protect from premature GC in cases where C sees no references to the ruby arena object (though upb may still be using the associated memory).

Results using script from #9467.

Before:

Encoded payload length: 1888895
Memory before test loop:    rss 121,432    vsz 1,253,468
Starting test loop...
1..200:  25.3 ms/rep    rss 1,440,180    vsz 3,083,840
201..400:  28.4 ms/rep    rss 2,765,192    vsz 4,926,608
401..600:  20.1 ms/rep    rss 4,090,208    vsz 6,769,592
601..800:  18.5 ms/rep    rss 5,415,224    vsz 8,612,568
801..1000:  18.9 ms/rep    rss 6,739,976    vsz 10,455,544
1001..1200:  18.9 ms/rep    rss 8,064,992    vsz 12,298,868
1201..1400:  18.8 ms/rep    rss 9,390,272    vsz 14,142,024
1401..1600:  18.4 ms/rep    rss 10,715,288    vsz 15,985,180
1601..1800:  18.8 ms/rep    rss 12,040,304    vsz 17,828,336
1801..2000:  19.0 ms/rep    rss 13,365,320    vsz 19,671,492
2001..2200:  19.1 ms/rep    rss 14,690,600    vsz 21,514,984
2201..2400:  18.7 ms/rep    rss 16,015,616    vsz 23,358,144
2401..2600:  18.6 ms/rep    rss 17,340,632    vsz 25,201,300
2601..2800:  19.9 ms/rep    rss 18,665,648    vsz 27,044,456
2801..3000:  18.9 ms/rep    rss 19,990,664    vsz 28,887,612
3001..3200:  18.9 ms/rep    rss 21,315,680    vsz 30,730,768
3201..3400:  18.6 ms/rep    rss 22,640,960    vsz 32,573,924
3401..3600:  18.5 ms/rep    rss 23,965,976    vsz 34,417,080
3601..3800:  18.6 ms/rep    rss 25,290,992    vsz 36,260,236
3801..4000:  18.8 ms/rep    rss 26,616,008    vsz 38,103,392
4001..4200:  18.5 ms/rep    rss 27,941,288    vsz 39,947,220
4201..4400:  18.0 ms/rep    rss 29,266,304    vsz 41,790,376
4401..4600:  18.2 ms/rep    rss 30,591,320    vsz 43,633,536
4601..4800:  17.3 ms/rep    rss 31,916,600    vsz 45,476,692
4801..5000:  19.4 ms/rep    rss 33,241,616    vsz 47,319,848
Total test time: 97.5 seconds
Memory after final GC:    rss 33,241,616    vsz 47,319,848

After (gem "google-protobuf", :path => "~/protobuf/ruby/"):

Encoded payload length: 1888895
Memory before test loop:    rss 43,976    vsz 1,168,080
Starting test loop...
1..200:  3.6 ms/rep    rss 134,880    vsz 1,260,180
201..400:  2.7 ms/rep    rss 134,880    vsz 1,260,180
401..600:  2.7 ms/rep    rss 134,880    vsz 1,260,180
601..800:  2.7 ms/rep    rss 134,880    vsz 1,260,180
801..1000:  2.9 ms/rep    rss 81,368    vsz 1,204,764
1001..1200:  2.9 ms/rep    rss 90,888    vsz 1,215,924
1201..1400:  2.9 ms/rep    rss 88,280    vsz 1,214,104
1401..1600:  2.9 ms/rep    rss 67,688    vsz 1,191,084
1601..1800:  3.0 ms/rep    rss 82,732    vsz 1,213,476
1801..2000:  3.0 ms/rep    rss 90,028    vsz 1,215,060
2001..2200:  3.0 ms/rep    rss 90,736    vsz 1,218,912
2201..2400:  2.9 ms/rep    rss 92,932    vsz 1,218,156
2401..2600:  3.0 ms/rep    rss 89,500    vsz 1,214,700
2601..2800:  3.0 ms/rep    rss 91,276    vsz 1,218,156
2801..3000:  3.6 ms/rep    rss 92,596    vsz 1,218,372
3001..3200:  4.0 ms/rep    rss 92,764    vsz 1,218,084
3201..3400:  3.3 ms/rep    rss 92,668    vsz 1,218,156
3401..3600:  3.5 ms/rep    rss 89,500    vsz 1,214,700
3601..3800:  3.9 ms/rep    rss 89,416    vsz 1,215,240
3801..4000:  4.1 ms/rep    rss 120,416    vsz 1,250,892
4001..4200:  3.2 ms/rep    rss 120,416    vsz 1,250,892
4201..4400:  2.7 ms/rep    rss 120,416    vsz 1,250,892
4401..4600:  2.7 ms/rep    rss 120,416    vsz 1,250,892
4601..4800:  2.7 ms/rep    rss 120,416    vsz 1,250,892
4801..5000:  2.7 ms/rep    rss 120,416    vsz 1,250,892
Total test time: 16.0 seconds
Memory after final GC:    rss 120,416    vsz 1,250,892

@zhangskz
zhangskz marked this pull request as ready for review March 4, 2022 22:19
@zhangskz
zhangskz requested a review from haberman March 4, 2022 22:19
@haberman

haberman commented Mar 4, 2022

Copy link
Copy Markdown
Member

Looks great! Could you put before/after measurements for the benchmark into the PR description, to make the impact even more clear?

zhangskz added 2 commits March 8, 2022 00:27
@zhangskz
Add RB_GC_GUARD to ensure ruby does not aggressively garbage collect …
d45843a 
…arena_rb due to lack of references, when memory is still used by file_proto
@zhangskz
Enable GC.stress in gc_test for ruby 2.7. This was previously disable…
f4fed16 
…d due to a ruby GC bug which has since been closed: https://bugs.ruby-lang.org/issues/16807
@zhangskz
Undo previous commit that enabled gc.stress for ruby 2.7. Though the …
79bb0af 
…issue is theoretically fixed (probably at a different version), Kokoro uses 2.7.0 and fails
@haberman haberman mentioned this pull request Mar 8, 2022
Closed
@zhangskz
zhangskz changed the base branch from master to 3.20.x March 8, 2022 17:03
@zhangskz
zhangskz merged commit 276add0 into protocolbuffers:3.20.x Mar 8, 2022
@haberman haberman mentioned this pull request Mar 8, 2022
Closed
@zhangskz zhangskz mentioned this pull request Mar 8, 2022
Closed
@zhangskz
zhangskz deleted the master branch April 1, 2022 18:58
@djudd djudd mentioned this pull request Jun 3, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haberman haberman haberman approved these changes

Assignees

No one assigned

Labels

ruby

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Ruby Arena object should use xrealloc/xfree so Ruby is aware of allocated memory

3 participants

@zhangskz @haberman @protobuf-kokoro