Skip to content

Commit

Permalink
Merge branch 'safe-result' into row-coder
Browse files Browse the repository at this point in the history
  • Loading branch information
@larskanis
larskanis committed Oct 4, 2019
2 parents e849383 + 2104e1d commit 07d353a
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 97 deletions.
10 changes: 1 addition & 9 deletions ext/pg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,6 @@ typedef struct {
/* Kind of PG::Coder object for casting COPY rows to ruby values */
VALUE decoder_for_get_copy_data;

/* enable/disable guessing size of PGresult's allocated memory */
int guess_result_memsize;

#if defined(_WIN32)
/* File descriptor to be used for rb_w32_unwrap_io_handle() */
int ruby_sd;
Expand Down Expand Up @@ -334,12 +331,7 @@ VALUE pg_tuple_new _(( VALUE, int ));
static inline t_pg_result *
pgresult_get_this( VALUE self )
{
t_pg_result *this = RTYPEDDATA_DATA(self);

if( this == NULL )
rb_raise(rb_ePGerror, "result has been cleared");

return this;
return RTYPEDDATA_DATA(self);
}


Expand Down
16 changes: 0 additions & 16 deletions ext/pg_connection.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ pgconn_s_allocate( VALUE klass )
this->decoder_for_get_copy_data = Qnil;
this->trace_stream = Qnil;
this->external_encoding = Qnil;
this->guess_result_memsize = 1;

return self;
}
Expand Down Expand Up @@ -4071,20 +4070,6 @@ pgconn_decoder_for_get_copy_data_get(VALUE self)
return this->decoder_for_get_copy_data;
}

/*
* call-seq:
* res.guess_result_memsize = enabled
*
* This method is for testing only and will probably be removed in the future.
*/
static VALUE
pgconn_guess_result_memsize_set(VALUE self, VALUE enable)
{
t_pg_connection *this = pg_get_connection( self );
this->guess_result_memsize = RTEST(enable);
return enable;
}


/*
* Document-class: PG::Connection
Expand Down Expand Up @@ -4214,7 +4199,6 @@ init_pg_connection()
rb_define_method(rb_cPGconn, "set_error_verbosity", pgconn_set_error_verbosity, 1);
rb_define_method(rb_cPGconn, "trace", pgconn_trace, 1);
rb_define_method(rb_cPGconn, "untrace", pgconn_untrace, 0);
rb_define_method(rb_cPGconn, "guess_result_memsize=", pgconn_guess_result_memsize_set, 1);

/****** PG::Connection INSTANCE METHODS: Notice Processing ******/
rb_define_method(rb_cPGconn, "set_notice_receiver", pgconn_set_notice_receiver, 0);
Expand Down
129 changes: 57 additions & 72 deletions ext/pg_result.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@

VALUE rb_cPGresult;

static void pgresult_gc_free( t_pg_result * );
static VALUE pgresult_type_map_set( VALUE, VALUE );
static VALUE pgresult_s_allocate( VALUE );
static t_pg_result *pgresult_get_this( VALUE );
static t_pg_result *pgresult_get_this_safe( VALUE );

Expand Down Expand Up @@ -105,6 +103,27 @@ pgresult_approx_size(const PGresult *result)
}
#endif

/*
* GC Mark function
*/
static void
pgresult_gc_mark( t_pg_result *this )
{
int i;

rb_gc_mark( this->connection );
rb_gc_mark( this->typemap );
rb_gc_mark( this->tuple_hash );
rb_gc_mark( this->field_map );

for( i=0; i < this->nfields; i++ ){
rb_gc_mark( this->fnames[i] );
}
}

/*
* GC Free function
*/
static void
pgresult_clear( t_pg_result *this )
{
Expand All @@ -115,15 +134,40 @@ pgresult_clear( t_pg_result *this )
#endif
}
this->result_size = 0;
this->nfields = -1;
this->pgresult = NULL;
}

static void
pgresult_gc_free( t_pg_result *this )
{
pgresult_clear( this );
xfree(this);
}

static size_t
pgresult_memsize( t_pg_result *this )
{
/* Ideally the memory 'this' is pointing to should be taken into account as well.
* However we don't want to store two memory sizes in t_pg_result just for reporting by ObjectSpace.memsize_of.
*/
return this->result_size;
}

static const rb_data_type_t pgresult_type = {
"pg",
{
(void (*)(void*))pgresult_gc_mark,
(void (*)(void*))pgresult_gc_free,
(size_t (*)(const void *))pgresult_memsize,
},
0, 0,
#ifdef RUBY_TYPED_FREE_IMMEDIATELY
RUBY_TYPED_FREE_IMMEDIATELY,
#endif
};


/*
* Global functions
*/
Expand All @@ -135,7 +179,7 @@ static VALUE
pg_new_result2(PGresult *result, VALUE rb_pgconn)
{
int nfields = result ? PQnfields(result) : 0;
VALUE self = pgresult_s_allocate( rb_cPGresult );
VALUE self;
t_pg_result *this;

this = (t_pg_result *)xmalloc(sizeof(*this) + sizeof(*this->fnames) * nfields);
Expand All @@ -146,7 +190,7 @@ pg_new_result2(PGresult *result, VALUE rb_pgconn)
this->nfields = -1;
this->tuple_hash = Qnil;
this->field_map = Qnil;
RTYPEDDATA_DATA(self) = this;
self = TypedData_Wrap_Struct(rb_cPGresult, &pgresult_type, this);

PG_ENCODING_SET_NOCHECK(self, ENCODING_GET(rb_pgconn));

Expand All @@ -169,18 +213,20 @@ pg_new_result(PGresult *result, VALUE rb_pgconn)
{
VALUE self = pg_new_result2(result, rb_pgconn);
t_pg_result *this = pgresult_get_this(self);
t_pg_connection *p_conn = pg_get_connection(rb_pgconn);

this->autoclear = 0;

if( p_conn->guess_result_memsize ){
/* Approximate size of underlying pgresult memory storage and account to ruby GC */
this->result_size = pgresult_approx_size(result);
/* Estimate size of underlying pgresult memory storage and account to ruby GC.
* There's no need to adjust the GC for xmalloc'ed memory, but libpq is using libc malloc() ruby doesn't know about.
*/
/* TODO: If someday most systems provide PQresultMemorySize(), it's questionable to store result_size in t_pg_result in addition to the value already stored in PGresult.
* For now the memory savings don't justify the ifdefs necessary to support both cases.
*/
this->result_size = pgresult_approx_size(result);

#ifdef HAVE_RB_GC_ADJUST_MEMORY_USAGE
rb_gc_adjust_memory_usage(this->result_size);
rb_gc_adjust_memory_usage(this->result_size);
#endif
}

return self;
}
Expand Down Expand Up @@ -315,37 +361,6 @@ pgresult_autoclear_p( VALUE self )
* DATA pointer functions
*/

/*
* GC Mark function
*/
static void
pgresult_gc_mark( t_pg_result *this )
{
int i;

if( !this ) return;
rb_gc_mark( this->connection );
rb_gc_mark( this->typemap );
rb_gc_mark( this->tuple_hash );
rb_gc_mark( this->field_map );

for( i=0; i < this->nfields; i++ ){
rb_gc_mark( this->fnames[i] );
}
}

/*
* GC Free function
*/
static void
pgresult_gc_free( t_pg_result *this )
{
if( !this ) return;
pgresult_clear( this );

xfree(this);
}

/*
* Fetch the PG::Result object data pointer and check it's
* PGresult data pointer for sanity.
Expand Down Expand Up @@ -376,33 +391,6 @@ pgresult_get(VALUE self)
}


static const rb_data_type_t pgresult_type = {
"pg",
{
(void (*)(void*))pgresult_gc_mark,
(void (*)(void*))pgresult_gc_free,
(size_t (*)(const void *))pgresult_memsize,
},
0, 0,
#ifdef RUBY_TYPED_FREE_IMMEDIATELY
RUBY_TYPED_FREE_IMMEDIATELY,
#endif
};

/*
* Document-method: allocate
*
* call-seq:
* PG::Result.allocate -> result
*/
static VALUE
pgresult_s_allocate( VALUE klass )
{
VALUE self = TypedData_Wrap_Struct( klass, &pgresult_type, NULL );

return self;
}

static void pgresult_init_fnames(VALUE self)
{
t_pg_result *this = pgresult_get_this_safe(self);
Expand Down Expand Up @@ -1449,8 +1437,7 @@ pgresult_stream_each_tuple(VALUE self)
void
init_pg_result()
{
rb_cPGresult = rb_define_class_under( rb_mPG, "Result", rb_cObject );
rb_define_alloc_func( rb_cPGresult, pgresult_s_allocate );
rb_cPGresult = rb_define_class_under( rb_mPG, "Result", rb_cData );
rb_include_module(rb_cPGresult, rb_mEnumerable);
rb_include_module(rb_cPGresult, rb_mPGconstants);

Expand Down Expand Up @@ -1507,5 +1494,3 @@ init_pg_result()
rb_define_method(rb_cPGresult, "stream_each_row", pgresult_stream_each_row, 0);
rb_define_method(rb_cPGresult, "stream_each_tuple", pgresult_stream_each_tuple, 0);
}


0 comments on commit 07d353a

Please sign in to comment.