From: "Glass_saga (Masaki Matsushita)" <glass.saga@...>
Date: 2013-05-17T22:59:45+09:00
Subject: [ruby-core:55027] [ruby-trunk - Feature #3620] Add Queue, SIzedQueue and ConditionVariable implementations in C in addition to ruby ones


Issue #3620 has been updated by Glass_saga (Masaki Matsushita).

File patch.diff added

I fixed some bugs:
(1) blocking forever bug pointed out by ko1 in [ruby-core:45950]
(2) SEGV in do_sleep()
(3) SizedQueue's bug which is similar to (1)

Now, the C implementation passes test-all.
Following diff is from final_queue_without_mutex.diff to my patch.

diff --git a/ext/thread/thread.c b/ext/thread/thread.c
index 9aff5e5..33365c1 100644
--- a/ext/thread/thread.c
+++ b/ext/thread/thread.c
@@ -150,7 +150,7 @@ static VALUE
 do_sleep(VALUE args)
 {
     struct sleep_call *p = (struct sleep_call *)args;
-    return rb_funcall(p->argv[0], rb_intern("sleep"), p->argc-1, p->argv+1);
+    return rb_funcall(p->argv[0], rb_intern("sleep"), p->argc-1, p->argv[1]); /* (2) */
 }

 static VALUE
@@ -339,15 +339,31 @@ rb_queue_push(VALUE self, VALUE obj)
     return self;
 }

+struct waiting_delete {
+    VALUE waiting;
+    VALUE th;
+};
+
+static VALUE
+queue_delete_from_waiting(struct waiting_delete *p)
+{
+    rb_ary_delete(p->waiting, p->th);
+    return Qnil;
+}
+
 static VALUE
 queue_do_pop(Queue *queue, VALUE should_block)
 {
-    while (!RARRAY_LEN(queue->que)) {
+    struct waiting_delete args;
+
+    while (RARRAY_LEN(queue->que) == 0) {
        if (!(int)should_block) {
            rb_raise(rb_eThreadError, "queue empty");
        }
-       rb_ary_push(queue->waiting, rb_thread_current());
-       rb_thread_sleep_forever();
+       args.waiting = queue->waiting;
+       args.th      = rb_thread_current();
+       rb_ary_push(args.waiting, args.th);
+       rb_ensure((VALUE (*)())rb_thread_sleep_forever, (VALUE)0, queue_delete_from_waiting, (VALUE)&args); /* (1) */
     }

     return rb_ary_shift(queue->que);
@@ -590,9 +606,14 @@ rb_szqueue_max_set(VALUE self, VALUE vmax)
 static VALUE
 szqueue_do_push(SizedQueue *szqueue, VALUE obj)
 {
+    struct waiting_delete args;
+    VALUE thread;
+
     while (queue_length(&szqueue->queue_) >= szqueue->max) {
-       rb_ary_push(szqueue->queue_wait, rb_thread_current());
-       rb_thread_sleep_forever();
+       args.waiting = szqueue->queue_wait;
+       args.th      = rb_thread_current();
+       rb_ary_push(args.waiting, args.th);
+       rb_ensure((VALUE (*)())rb_thread_sleep_forever, (VALUE)0, queue_delete_from_waiting, (VALUE)&args); /* (3) */
     }
     return queue_do_push(&szqueue->queue_, obj);
 }
----------------------------------------
Feature #3620: Add Queue, SIzedQueue and ConditionVariable implementations in C in addition to ruby ones
https://bugs.ruby-lang.org/issues/3620#change-39373

Author: panaggio (Ricardo Panaggio)
Status: Assigned
Priority: Normal
Assignee: ko1 (Koichi Sasada)
Category: ext
Target version: next minor


=begin
 Queue, SizedQueue and ConditionVariable are important synchronization primitives and are nowadays implemented in Ruby.
 
 Attached patch (initiated by myself and heavily enriched by Nobu) contains these sync primitives implemented in C, which makes them faster (see [1] for the benchmark's code):
 
 Rehearsal -------------------------------------------------
 Q#push          1.590000   0.010000   1.600000 (  1.605502)
 T#push          0.600000   0.010000   0.610000 (  0.630444)
 Q#pop           4.390000   0.000000   4.390000 (  4.389781)
 T#pop           0.580000   0.000000   0.580000 (  0.578918)
 Q#empty?        0.480000   0.000000   0.480000 (  0.484305)
 T#empty?        0.360000   0.000000   0.360000 (  0.358559)
 Q#clear         1.210000   0.000000   1.210000 (  1.214494)
 T#clear         0.600000   0.000000   0.600000 (  0.588611)
 Q#size          0.370000   0.000000   0.370000 (  0.365587)
 T#size          0.350000   0.000000   0.350000 (  0.356985)
 Q#num_waiting   0.380000   0.000000   0.380000 (  0.379199)
 T#num_waiting   0.370000   0.000000   0.370000 (  0.368075)
 --------------------------------------- total: 11.300000sec
 
 It has already been discussed on ruby-core (see ruby-core:31100).
 
 This patch is one of the deliverables of my RubySoC project (slot #17): "Improving Ruby's Synchronization Primitives and Core Libraries" [2,3]
 
 [1] http://github.com/panaggio/rubysoc-2010/blob/master/benchmarks/queue.rb
 [2] http://pastebin.com/viSnfqe6
 [3] http://rubysoc.org/projects
=end



-- 
http://bugs.ruby-lang.org/