why is imp_xxh stored in an SV?

2012-02-24 Thread Dave Mitchell
(I confess I haven't looked closely at this yet, but...)

I seems that the imp_xxh_t structure is stored in the PVX of an SV
pointed to by mg_obj. Is there any reason why it can't be just directly
malloced and pointed to from mg_ptr instead?


-- 
The warp engines start playing up a bit, but seem to sort themselves out
after a while without any intervention from boy genius Wesley Crusher.
-- Things That Never Happen in "Star Trek" #17


Misc DBI_dispatch speedup patches

2012-02-24 Thread Dave Mitchell
The 8 attached patches (to be applied in sequence) all provide tweaks,
cleanup, or otherwise attempt to make DBI_dispatch go a little bit faster.
This speedups are all relatively minor, and my timings show the
cumulative speedup for my standard fetch() test loop is somewhere between
0% and 9% based on the perl version (better for newer perls), but
generally with a variation of around 5% between test runs. So take with a
pinch of salt.

These patches should be possible to apply without my DBIS patch having
been applied (although some hand merging might be necessary).

The first patch is the biggest. It moves the method lookup cache into the
dbi_ima_t struct (rather than being directly attached to magic).
This is what you originally suggested I do, but I couldn't do it because
the any_ptr field in the CV wouldn't be properly duped or freed.
I subsequently realised that I could get round this by attaching magic
(with dup and free functions), which administers to the any_ptr field.
This makes things a lot simpler.
It was then made more complex again by the fact that in perls < 5.8.9, the
new any_ptr field is overwritten after the magic dup handler is called :-(
So there's some special ifdef'd code to handle this.

This patch also means that *all* CVs are given dbi_ima_t structure when
installed, rather than only the ones with attributes.

The second patch just inlines the method cache lookup, now that it's
simpler.

The third makes use of ima always being available, by pre-calculating
type of method at install time; i.e. this

*meth_name =='F' && strEQ(meth_name,"FETCH")

becomes

meth_type == methtype_FETCH

The fourth does a shortcut for mg_find, similar to that done in
dbih_getcom2().

The fifth follows on from that by eliminating the is_FETCH var.

The sixth slightly tweaks where qsv (quick SV) is tested.

The seventh removes some GV tests relating to the method to be called.
It used to do isGV(imp_msv) before accessing  GvCV(imp_msv); but since an
earlier bit of code accesses GvCV(imp_msv) before the test, I assume
that either the test is redundant and imp_msv is always a gv, or
that it might not be a GV and it's just a miracle that the code hasn't
SEGVed in the past (if the latter, then my patch is wrong).

The eighth optimises the stack handling done before/after the inner method
call in the XS case.

Dave


-- 
print+qq&$}$"$/$s$,$a$d$g$s$@$.$q$,$:$.$q$^$,$@$a$~$;$.$q$m&if+map{m,^\d{0\,},,${$::{$'}}=chr($"+=$&||1)}q&10m22,42}6:17a2~2.3@3;^2dg3q/s"&=~m*\d\*.*g
>From 7b75e81af1d8563dce002f23ead618150749e592 Mon Sep 17 00:00:00 2001
From: David Mitchell 
Date: Mon, 20 Feb 2012 18:38:13 +
Subject: [PATCH 1/8] move method cache into dbi_ima_t struct

The recently-added method cache stored the cache in magic attached to the
outer CV, rather than in the dbi_ima_t structure also attached to the CV,
because the dbi_ima_t structure couldn't be duped or freed, and so
couldn't have reference-counted SVs stored in it.

Change it so that the cache is now stored in the dbi_ima_t struct; but
still attach magic, who's sole function is to provide dup() and free()
methods which administer the dbi_ima_t struct.

This makes the code simpler, and also means that at method lookup time,
we don't have to retrieve any structures from magic.

Note that with this change, every DBI_dispatch method has a dbi_ima_t
struct, not just those with attributes. This will allow us to make further
use of it in the future.

There's a wart however. In thread cloning in perls before 5.8.9,
the any_ptr is copied from the old to the new CV *after* our magic dup
function is called, thus stopping us from being able to duplicate the
dbi_ima_t structure. We work round this (in code clearly delineated with
the BROKEN_DUP_ANY_PTR macro) by instead storing the current perl
interpreter address in in the dbi_ima_t; then in XS_DBI_dispatch,
we duplcate the dbi_ima_t at that point if the my_perl's don't match.
Clumsy, but should still be more efficient than the previous method of
storing the cache.

Once support for 5.8.x is dropped, it will be easy to remove this extra
code.
---
 DBI.xs |  184 
 1 files changed, 103 insertions(+), 81 deletions(-)

diff --git a/DBI.xs b/DBI.xs
index 27b93b4..217e314 100644
--- a/DBI.xs
+++ b/DBI.xs
@@ -80,6 +80,14 @@ extern Pid_t getpid (void);
 #define DBI_save_hv_fetch_ent
 #endif
 
+/* prior to 5.8.9: when a CV is duped, the mg dup method is called,
+ * then *afterwards*, any_ptr is copied from the old CV to the new CV.
+ * This wipes out anything which the dup method did to any_ptr.
+ * This needs working around */
+#if defined(USE_ITHREADS) && (PERL_VERSION == 8) && (PERL_SUBVERSION < 9)
+#  define BROKEN_DUP_ANY_PTR
+#endif
+
 
 static imp_xxh_t *dbih_getcom  _((SV *h));
 static imp_xxh_t *dbih_getcom2 _((pTHX_ SV *h, MAGIC **mgp));
@@ -100,8 +108,10 @@ static int  sql_type_cast_svpv _((pTHX_ SV *sv, int 
sql_type, U32 flags, voi