From: Charles Mills <cmills@...>
Date: 2005-07-22T06:36:00+09:00
Subject: Re: C extension makes things slower

Phil Tomson wrote:
> In article <20050721175902.GH2279@tux-chan>,
> Mauricio Fern�ndez  <mfp@acm.org> wrote:
> >On Fri, Jul 22, 2005 at 02:05:56AM +0900, Phil Tomson wrote:
> >> In article <42DF4F1D.2060607@capitain.de>,
> >> Pit Capitain  <pit@capitain.de> wrote:
> >> >Phil Tomson schrieb:
> >> >> And the results:
> >> >> l% ruby point.rb
> >> >>   Total time for pure ruby point test: 8.600752
> >> >>   ACO_Ext initializing...
> >> >>   Total time for extension point test: 6.086567
> >> However, here's the result of another run:
> >>
> >>   Total time for pure ruby point test: 8.051388
> >>   ACO_Ext initializing...
> >>   Total time for extension point test: 9.212984
> >
> >You're doing things like
> >
> >static VALUE aco_point_diff(VALUE self, VALUE other){
> >  double self_x = NUM2DBL(rb_iv_get(self, "@x"));
> >  double self_y = NUM2DBL(rb_iv_get(self, "@y"));
> >  double other_x= NUM2DBL(rb_iv_get(other, "@x"));
> >  double other_y= NUM2DBL(rb_iv_get(other, "@y"));
> >  double xdiffsq= pow( (self_x - other_x),2 );
> >  double ydiffsq= pow( (self_y - other_y),2 );
> >  return rb_float_new(sqrt( xdiffsq  +  ydiffsq ) );
> >
> >}
> >
> >when you probably want something resembling
> >
> >typedef struct {
> >        double x;
> >        double y;
> >} ACO_Point_Struct;
> >
> >static VALUE
> >aco_point_diff(VALUE self, VALUE other)
> >{
> >  ACO_Point_Struct *_self, *_other;
> >  double dx, dy;
> >
> >  Data_Get_Struct(self, ACO_Point_Struct, _self);
> >  Data_Get_Struct(other, ACO_Point_Struct, _other);
> >
> >  dx = _self->x - _other->x;
> >  dy = _self->y - _other->y;
> >  return rb_float_new(sqrt(dx * dx + dy * dy));
> >}
> >
>
> Thanks, that's _MUCH_ (and consistently) faster:
>
>   $ ruby point.rb
>   Total time for pure ruby point test: 6.977688
>   ACO_Ext initializing...
>   Total time for extension point test: 2.777282
>
>
> And now my real application that uses point is much faster too:
>   $ruby TSP.rb
>   -->best tour in gen 0: 8667.97295111485
>   -->global best tour is: 8128.7749762166
>   Total time: 3.635439
>
> ...used to be something like 22 seconds.
>
> Oh, for the sake of complete documentation in case someone has similar
> questions in the future:
>
> I had to add an allocate function so here's the new version of aco_ext.c:
>
> //aco_ext.c
> #include "ruby.h"
> #include <math.h>
> #include <stdio.h>
>
> static int id_x;
> static int id_y;
> static int id_equal;
>
> typedef struct {
>         double x;
>         double y;
> } ACO_Point_Struct;
>
> static void aco_point_free(void* p){
>   free(p);
> }
>
> static VALUE aco_point_alloc(VALUE klass) {
>   VALUE object;
>   ACO_Point_Struct* point = (ACO_Point_Struct*)malloc(sizeof(
> ACO_Point_Struct));
>   object = Data_Wrap_Struct(klass,0,aco_point_free,point);
>   return object;
> }
>
> static VALUE aco_point_init(VALUE self, VALUE x, VALUE y){
>   ACO_Point_Struct* _self ;
>   //double x, y;
>   Data_Get_Struct(self, ACO_Point_Struct, _self);
>   _self->x = NUM2DBL(x);
>   _self->y = NUM2DBL(y);
>   return self;
> }
>
> static VALUE
> aco_point_diff(VALUE self, VALUE other)
> {
>   ACO_Point_Struct *_self, *_other;
>   double dx, dy;
>
>   Data_Get_Struct(self, ACO_Point_Struct, _self);
>   Data_Get_Struct(other, ACO_Point_Struct, _other);
>
>   dx = _self->x - _other->x;
>   dy = _self->y - _other->y;
>   return rb_float_new(sqrt(dx * dx + dy * dy));
> }
>
> static VALUE aco_point_equal(VALUE self, VALUE other){
>   ACO_Point_Struct *_self;
>   ACO_Point_Struct *_other;
>   double x, y;
>   Data_Get_Struct(self, ACO_Point_Struct, _self);
>   Data_Get_Struct(other, ACO_Point_Struct, _other);
>   return((_self->x == _other->x) && (_self->y == _other->y));
> }
>
> VALUE cACOMod;
> VALUE cACOPoint;
> VALUE cACOGraph;
>
> void Init_ACO_Ext() {
>   printf("ACO_Ext initializing...\n");
>   cACOMod = rb_define_module("ACO");
>   cACOPoint = rb_define_class_under(cACOMod,"Point",rb_cObject);
>   cACOGraph = rb_define_class_under(cACOMod,"Graph",rb_cObject);
>   rb_define_alloc_func(cACOPoint, aco_point_alloc);
>   rb_define_method(cACOPoint,"initialize",aco_point_init,2);
>   rb_define_method(cACOPoint,"-",aco_point_diff,1);
>   rb_define_method(cACOPoint,"==",aco_point_equal,1);
>
>   id_x      = rb_intern("x");
>   id_y      = rb_intern("y");
>   id_equal  = rb_intern("==");
> }
> //end aco_ext.c
>
>
> Phil

As it stands right now if you do something like:
point - "hey"
you will probably crash Ruby or worse.

Another option would be to do (untested/uncheck code):

#define IsACO_Point(v) (TYPE(v) == T_DATA && RDATA(v)->dfree ==
aco_point_free)

static ACO_Point_Struct
aco_to_aco(VALUE other)
{
	ACO_Point_Struct p;
  if (IsACO_Point(other)) {
    ACO_Point_Struct *p_ptr;
    Data_Get_Struct(self, ACO_Point_Struct, p_ptr);
    p = *p_ptr;
  } else {
    VALUE x, y;
    x = rb_convert_type(T_FLOAT, rb_funcall2(other, id_x, 0, 0),
"to_f", "Float");
    y = rb_convert_type(T_FLOAT, rb_funcall2(other, id_y, 0, 0),
"to_f", "Float");
    p.x = RFLOAT(x)->value;
    p.y = RFLOAT(y)->value;
  }
  return p;
}

then use aco_to_aco() whenever your not sure of the type/class of an
argument (ie when it is not self).
doing this also leaves duck typing options open :)

-Charlie