On 2/22/07, John L. [email protected] wrote:
Thanks Dave! Looking forward to it.
Can you tell us a bit more about what led to the segfault error cropping
up? Have they been in the 0.10 branch all along? 0.9 too? Or did some
new work break something?
Maybe it will help others debug problems in future.
Well, the main problem I fixed was due to an error introduced in 0.10.
I wasn’t locking the commit log in all the places I should have. This
actually would have been very easy to fix if someone had supplied a
repeatable test case. In the end though I decided to lock-less
commits, a new feature that has recently been added to Lucene. The
main advantages of this are that you can open IndexReaders when an
IndexWriter is committing and you can open multiple IndexReaders at a
time without them interrupting each other. It also makes it much
easier to recover after a crash. If your system crashes in the middle
of a commit then Ferret will be able to open the previously committed
version of the index.
As for the segfaults, I think I finally found the problem today. To
improve the performance of Ferret’s bindings I was adding objects to
Ruby’s Array directly instead of using the rb_ary_push method. Some of
these arrays are quite large so using rb_ary_push was a lot of
overhead which I didn’t think was really necessary … but I didn’t
quite get it right. For example, I had;
rterms = rb_ary_new2(term_cnt);
rts = RARRAY(rterms)->ptr;
RARRAY(rterms)->len = term_cnt;
for (i = 0; i < term_cnt; i++) {
rts[i] = frt_get_tv_term(&terms[i]);
}
So, in this example, the number of terms in a field can be very large
and we save a lot of time[1] by setting the C array directly rather
than use rb_ary_push. The problem occurs when the garbage collector
gets called in the middle of filling the array. It will try and mark
all of the objects contained by the array but the array isn’t filled
yet so many of its elements haven’t been set yet. What I should have
done was incremented the array length as I went.
rterms = rb_ary_new2(term_cnt);
rts = RARRAY(rterms)->ptr;
for (i = 0; i < term_cnt; i++) {
rts[i] = frt_get_tv_term(&terms[i]);
RARRAY(rterms)->len++;
}
This is touch slower than the original code but it now works so that’s
all that matters. You may be thinking I could have just set the length
after the loop.
rterms = rb_ary_new2(term_cnt);
rts = RARRAY(rterms)->ptr;
for (i = 0; i < term_cnt; i++) {
rts[i] = frt_get_tv_term(&terms[i]);
}
RARRAY(rterms)->len = term_cnt;
But the problem here is that the elements that have been added to the
array won’t actually get marked by the garbage collector because the
array’s length is still 0 so the could incorrectly be collected, thus
also causing a segfault. One alternate method that will work would be
to user rb_mem_clear():
rterms = rb_ary_new2(term_cnt);
rb_mem_clear(rterms, term_cnt); // initialize all elements to nil
rts = RARRAY(rterms)->ptr;
RARRAY(rterms)->len = term_cnt;
for (i = 0; i < term_cnt; i++) {
rts[i] = frt_get_tv_term(&terms[i]);
}
This makes sure all elements are set to nil before the are set to the
term vector so they are therefor safe from the garbage collector.
Anyway, sorry for the long and boring post. I guess the point is to
think about how the garbage collector works when developing ruby
bindings.
Cheers,
Dave
[1] How much faster? About 20% faster according to a simple benchmark
I just ran. Was it worth the segfaults? Of course not but in a library
like this you take the optimizations where you can get them.
If not, perhaps you could overwrite the mark function for the