Lorenz Minder on Tue, 25 Aug 2009 00:16:47 +0200


[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

Re: TRY/CATCH is unnecessarily slow on OS X


Hi,

Lorenz Minder wrote:
> Why is it so slow?  The problem is that the Mac OS X version of 
> setjmp/longjmp saves and restores the signal mask, which appears to take 
> an insane amount of time.  Assuming that this is not necessary (if it 
> is, I think you have a bug on SysV), it is better to use a variant of 
> setjmp/longjmp that does not do this. Use _setjmp/_longjmp or possibly 
> sigsetjmp(,0)/siglongjmp() instead.

I wrote a new patch (attached) that replaces setjmp/longjmp by
sigsetjmp(,0)/siglongjmp if the latter is available.  The patch defines
macros pari_setjmp()/pari_longjmp()/pari_jmp_buf which map to the right
functions and are intended for use when interacting with libpari.  This
seems to work ok for me.

I see two potential problems with this patch, though.  The first one is
that it incompatibly changes the err_catch() function (and it does so in
a way that is not necessarily compile-time detected).  I don't know what
the compatibility requirements of PARI are;  I can't find any
documentation of err_catch(), so I'm not sure if its users are supposed
to be on their own if this interface changes, or if it is maybe
necessary to introduce a new err_catch2() function, and leave the old
function as is.  (It would certainly be possible to keep track of
whether a stored jump buffer is a sigjmp_buf or a plain jmp_buf.)

The other possible problem is that error handling is supposed to be
reworked in any case if I understand correctly.  Unless I'm misreading
the plan, this would likely need a new API anyways, so it might be a
better plan just to change the API just once.  If that is the case, what
would be the best thing for me to do to ensure that the new API does not
have the same performance problems on BSD-style platforms as the current
one does?

Best regards,
--Lorenz
diff --git a/config/get_libc b/config/get_libc
--- a/config/get_libc
+++ b/config/get_libc
@@ -16,6 +16,7 @@
   esac;
 fi; . ./look
 list=sigaction; . ./look
+list=sigsetjmp; . ./look
 list=TIOCGWINSZ; . ./look
 list=getrlimit; . ./look
 list='stat opendir'; . ./look
diff --git a/config/has_sigsetjmp.c b/config/has_sigsetjmp.c
new file mode 100644
--- /dev/null
+++ b/config/has_sigsetjmp.c
@@ -0,0 +1,3 @@
+#include <setjmp.h>
+
+int main() { sigjmp_buf b; sigsetjmp(b, 0); return 0; }
diff --git a/config/paricfg.h.SH b/config/paricfg.h.SH
--- a/config/paricfg.h.SH
+++ b/config/paricfg.h.SH
@@ -160,6 +160,10 @@
 yes) echo '#define HAS_SIGACTION' >> $file;;
 esac
 
+case $has_sigsetjmp in
+yes) echo '#define HAS_SIGSETJMP' >> $file;;
+esac
+
 case $has_waitpid in
 yes) echo '#define HAS_WAITPID' >> $file;;
 esac
diff --git a/src/gp/gp.c b/src/gp/gp.c
--- a/src/gp/gp.c
+++ b/src/gp/gp.c
@@ -1270,7 +1270,7 @@
   b = filtered_buffer(&F);
   for(;;)
   {
-    if (setjmp(GP_DATA->env)) fprintferr("...skipping line %ld.\n", c);
+    if (pari_setjmp(GP_DATA->env)) fprintferr("...skipping line %ld.\n", c);
     c++;
     if (!get_line_from_file(NULL,&F,file)) break;
     s = b->buf;
@@ -1502,7 +1502,7 @@
       outtyp = GP_DATA->fmt->prettyp;
       tloc = H->total; recover(0);
       /* recover: jump from error [ > 0 ] or allocatemem [ -1 ] */
-      if ((er = setjmp(GP_DATA->env)))
+      if ((er = pari_setjmp(GP_DATA->env)))
       {
         if (ismain && er > 0) {
           char *s = (char*)global_err_data;
@@ -1673,7 +1673,7 @@
 read_main(const char *s)
 {
   GEN z;
-  if (setjmp(GP_DATA->env))
+  if (pari_setjmp(GP_DATA->env))
     z = NULL;
   else {
     switchin(s);
@@ -1821,7 +1821,7 @@
   } else if (initrc)
   {
     gp_initrc(p_A, argv[0]);
-    if (setjmp(GP_DATA->env))
+    if (pari_setjmp(GP_DATA->env))
     {
       puts("### Errors on startup, exiting...\n\n");
       exit(1);
@@ -1853,7 +1853,7 @@
   pari_stack s_A, *newfun, *oldfun;
 
   GP_DATA = default_gp_data();
-  if (setjmp(GP_DATA->env))
+  if (pari_setjmp(GP_DATA->env))
   {
     puts("### Errors on startup, exiting...\n\n");
     exit(1);
diff --git a/src/headers/paricom.h b/src/headers/paricom.h
--- a/src/headers/paricom.h
+++ b/src/headers/paricom.h
@@ -42,8 +42,8 @@
 #define CATCH(err) {         \
   VOLATILE long __err = err, __catcherr = -1; \
   int pari_errno;            \
-  jmp_buf __env;             \
-  if ((pari_errno = setjmp(__env)))
+  pari_jmp_buf __env;        \
+  if ((pari_errno = pari_setjmp(__env)))
 
 #define RETRY { __catcherr = err_catch(__err, &__env);
 #define TRY else RETRY
diff --git a/src/headers/paridecl.h b/src/headers/paridecl.h
--- a/src/headers/paridecl.h
+++ b/src/headers/paridecl.h
@@ -1813,7 +1813,7 @@
 void    dbg_gerepile(pari_sp av);
 void    dbg_gerepileupto(GEN q);
 void    dbg_release(void);
-long    err_catch(long errnum, jmp_buf *penv);
+long    err_catch(long errnum, pari_jmp_buf *penv);
 void    err_leave(long n);
 GEN     gclone(GEN x);
 GEN     gcopy(GEN x);
diff --git a/src/headers/paripriv.h b/src/headers/paripriv.h
--- a/src/headers/paripriv.h
+++ b/src/headers/paripriv.h
@@ -312,7 +312,7 @@
 
 /* GP_DATA */
 typedef struct {
-  jmp_buf env;
+  pari_jmp_buf env;
   gp_hist *hist;
   gp_pp *pp;
   gp_path *path;
@@ -331,7 +331,7 @@
 typedef struct Buffer {
   char *buf;
   ulong len;
-  jmp_buf env;
+  pari_jmp_buf env;
 } Buffer;
 Buffer *new_buffer(void);
 void delete_buffer(Buffer *b);
diff --git a/src/headers/parisys.h b/src/headers/parisys.h
--- a/src/headers/parisys.h
+++ b/src/headers/parisys.h
@@ -69,6 +69,16 @@
 #  define THREAD
 #endif
 
+#ifdef HAS_SIGSETJMP
+#define pari_setjmp(x) sigsetjmp((x), 0)
+#define pari_longjmp siglongjmp
+#define pari_jmp_buf sigjmp_buf
+#else
+#define pari_setjmp setjmp
+#define pari_longjmp longjmp
+#define pari_jmp_buf jmp_buf
+#endif
+
 #if defined(_WIN32) || defined(__CYGWIN32__)
 /* ANSI C does not allow to longjmp() out of a signal handler, in particular,
  * the SIGINT handler. On Win32, the handler is executed in another thread, and
diff --git a/src/language/eval.c b/src/language/eval.c
--- a/src/language/eval.c
+++ b/src/language/eval.c
@@ -95,7 +95,7 @@
   closure_reset();
   (void)allocatemoremem(newsize);
   global_err_data = NULL;
-  longjmp(GP_DATA->env, -1);
+  pari_longjmp(GP_DATA->env, -1);
 }
 
 /*******************************************************************/
diff --git a/src/language/init.c b/src/language/init.c
--- a/src/language/init.c
+++ b/src/language/init.c
@@ -69,7 +69,7 @@
 void (*cb_pari_sigint)(void);
 
 typedef struct {
-  jmp_buf *penv;
+  pari_jmp_buf *penv;
   long flag;
 } cell;
 
@@ -669,7 +669,7 @@
     pari_init_defaults();
   }
 
-  if ((init_opts&INIT_JMPm) && setjmp(GP_DATA->env)) pari_exit();
+  if ((init_opts&INIT_JMPm) && pari_setjmp(GP_DATA->env)) pari_exit();
   if ((init_opts&INIT_SIGm)) pari_sig_init(pari_sighandler);
   pari_init_stack(parisize, 0);
   diffptr = initprimes(maxprime);
@@ -780,7 +780,7 @@
 }
 
 long
-err_catch(long errnum, jmp_buf *penv)
+err_catch(long errnum, pari_jmp_buf *penv)
 {
   long n;
   /* for fear of infinite recursion */
@@ -824,7 +824,7 @@
 
   /* reclaim memory stored in "blocs" */
   if (try_to_recover) recover(1);
-  longjmp(GP_DATA->env, numerr);
+  pari_longjmp(GP_DATA->env, numerr);
 }
 
 static void
@@ -930,7 +930,7 @@
           global_err_data = (char*)va_arg(ap, char*);
           break;
       }
-      longjmp(*(trapped->penv), numerr);
+      pari_longjmp(*(trapped->penv), numerr);
     }
   }
   err_init();