Testing Yura's array-as-queue optimizations

26 views
Skip to first unread message

Jon

unread,
Nov 3, 2012, 9:46:59 PM11/3/12
to theco...@googlegroups.com
I did a one line tweak to Yura's array optimization `174.patch` from his feature request at

https://bugs.ruby-lang.org/issues/6638#note-7

to get it to apply to trunk@r37463. The tweaked patch is at https://gist.github.com/4009735

C:\Jenkins\workspace\ruby-trunk-svn>svn log -l 1
------------------------------------------------------------------------
r37463 | nobu | 2012-11-03 21:19:11 -0400 (Sat, 03 Nov 2012) | 5 lines

dir.c: FNM_EXTGLOB

* dir.c (file_s_fnmatch): match with expanding braces if FNM_EXTGLOB
  is set.  [ruby-core:40037] [Feature #5422]

------------------------------------------------------------------------

C:\Jenkins\workspace\ruby-trunk-svn>git apply --check --verbose array_as_queue_trunk.patch

Checking patch array.c...
Hunk #2 succeeded at 447 (offset -2 lines).
Hunk #3 succeeded at 2179 (offset -2 lines).
Hunk #4 succeeded at 2202 (offset -2 lines).
Hunk #5 succeeded at 2217 (offset -2 lines).
Checking patch array.c...
Hunk #2 succeeded at 770 (offset -2 lines).
Hunk #3 succeeded at 786 (offset -2 lines).
Hunk #4 succeeded at 810 (offset -2 lines).
Checking patch array.c...
Hunk #1 succeeded at 1372 (offset -2 lines).
Hunk #2 succeeded at 1387 (offset -2 lines).
Checking patch array.c...
Hunk #1 succeeded at 968 (offset -2 lines).
Hunk #2 succeeded at 1032 (offset -2 lines).

C:\Jenkins\workspace\ruby-trunk-svn>echo %errorlevel%
0


Anyone have the time and/or interest for testing this patch against trunk via `make test` and `make test-all` on Windows and Linux and giving nobu and mame feedback?  Any real-world array-as-queue code to benchmark the patch against and show ruby-core the results?

  http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/48829

Jon

Jon

unread,
Nov 3, 2012, 10:48:18 PM11/3/12
to theco...@googlegroups.com
Initial `make test` and `make test-all` success on trunk@37463

https://bugs.ruby-lang.org/issues/6638#note-8

If you've got tests (pass or fail) and/or benchmarks please don't hesitate to summarize the results on the feature request.

Jon

---
Fail fast. Fail often. Fail publicly. Learn. Adapt. Repeat.
http://thecodeshop.github.com | http://jonforums.github.com/
twitter: @jonforums

Funny Falcon

unread,
Nov 4, 2012, 1:07:19 AM11/4/12
to theco...@googlegroups.com
What tweak is? Which line did you change and why?

воскресенье, 4 ноября 2012 г., 5:47:00 UTC+4 пользователь Jon написал:

Funny Falcon

unread,
Nov 4, 2012, 5:53:24 AM11/4/12
to theco...@googlegroups.com
I've rebased pull request https://github.com/ruby/ruby/pull/174 against trunk

воскресенье, 4 ноября 2012 г., 10:07:19 UTC+4 пользователь Funny Falcon написал:

Jon

unread,
Nov 4, 2012, 8:37:57 AM11/4/12
to theco...@googlegroups.com
> What tweak is? Which line did you change and why?

Below is the hunk diff from my change.

I replaced the original `OBJSETUP(shared, 0, T_ARRAY)` with `NEWOBJ_OF(shared, struct RArray, 0, T_ARRAY)` so the patch would apply. No other change. Seems `NEWOBJ_OF` is the latest way of doing things in array.c

I just saw your rebase update to the issue, but your https://github.com/ruby/ruby/pull/174.patch is still the old one because the GH repo hasn't been synced with svn. This hunk tweak is still needed.


@@ -43,7 +43,7 @@ index 8b2c4c5..516e919 100644
VALUE *ptr = ALLOC_N(VALUE, len);
MEMCPY(ptr, RARRAY_PTR(ary), VALUE, len);
@@ -440,8 +449,9 @@
- OBJSETUP(shared, 0, T_ARRAY);
+ NEWOBJ_OF(shared, struct RArray, 0, T_ARRAY);
FL_UNSET_EMBED(shared);

Юрий Соколов

unread,
Nov 4, 2012, 10:12:56 AM11/4/12
to theco...@googlegroups.com
Thank you, John. I'll try to fix it next days.
It's a pitty that ruby core do not use git as a primary repo :(

2012/11/4 Jon <jon.f...@gmail.com>

Jon

unread,
Nov 5, 2012, 11:49:18 AM11/5/12
to theco...@googlegroups.com
> > Anyone have the time and/or interest for testing this patch against trunk
> > via `make test` and `make test-all` on Windows and Linux and giving nobu
> > and mame feedback? Any real-world array-as-queue code to benchmark the
> > patch against and show ruby-core the results?
> >
> > http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/48829

Yura,

I believe the following will help convince nobu and mame that your optimization should be committed in time for preview2. I'm concerned that if it doesn't get committed in preview2, it will never be in 2.0.0.

1) Summarize a compelling use case. Your optimization seems applicable to many areas (eventing, actors, and producer/consumer, etc) that use (or could use) in-memory queuing as part of the architecture. Providing one specific use case in the feature request would be more persuasive. For example, what use case caused you to start working on the optimization?

2) Provide a benchmark. The feature request mentions dramatic performance improvements, but currently no results are provided. I'm concerned that without specific results, nobu and mame don't have enough info to clearly see the performance advantages.

3) Add additional tests. It's fantastic the patch currently passes all known ruby-core tests on Ubuntu and Win7 32bit, but since ruby-core is focused on stabilizing trunk for the 2.0.0 release, I believe that any patch perceived as destabilizing will be rejected. Adding a few tests to your pull request will help "prove" stability and show "low risk." For example, when you were creating the patch, what areas worried you the most? What implementation edge cases should have new tests?

I'm sorry I can't jump in and help more with benchmarking or writing tests. Business and business travel is sucking all my time until about mid-December. That said, I will continue to run test and test-all with your patch whenever I rebuild trunk.

Jon
Reply all
Reply to author
Forward
0 new messages