Hello,
W dniu 31 sierpnia 2010 00:39 użytkownik Sven Neumann <sven@xxxxxxxx> napisał:
On Mon, 2010-08-30 at 23:31 +0200, Łukasz Czerwiński wrote:Thanks for your patch. It definitely needs some work though. First of
> I have made several optimizations in loading script-fu extension
> making the loading process a bit faster. It's the first time I change
> something in an open source program and don't know best way to
> "announce" it and get it reviewed, so I send a patch in a mail.
> The major speedup is caused by calling my function load_script_file
> instead of script_fu_run_command("(load ...."). Please review my
> changes and send an answer if the patch is OK.
all it is not OK to comment out code. If code should be removed, then
please remove it.
Ok, I will remove.
Passing a string literal directly to g_message() is also not a good
idea. Please use g_message("%s", message) instead. But I didn't
understand what this part of your patch is trying to achieve.
Why it's not a good idea? What's the difference? Nevertheless, ok, I'll change it.
Overall it would help a lot if you could explain what your patch tries
to achieve and what the purpose of the changes are. I have not been able
to understand the benefits of your changes just by looking at the patch.
The difference is clearly visible on a callgrind's graph. Compare flow between script_fu_run and Eval_Cycle at http://students.mimuw.edu.pl/~lc277642/gimp/before.jpg and http://students.mimuw.edu.pl/~lc277642/gimp/after.jpg.
Full versions of the graphs in jpg and Callgrind formats are available at http://students.mimuw.edu.pl/~lc277642/gimp/.
A description of an optimization: all script-fu scripts loaded on startup was loaded using a command "(load ...)" of a TinyScheme language. This means running several functions escaping a path to a script, interpreting the string, choping it to tokens, analyzing, unescaping and finally loading the script. My optimization makes Gimp call directly a function loading a script without using a slow (comparing to calling one function in native C) TinyScheme interpreter. Instead I've written two helper functions (load_script_file and scheme_file_push) responsible for loading scripts.
I have done also a second modification - connected with comparing strings (stricmp), but I've just spotted an error in it, so I'll fix it and send it later.
Is my description detailed enough or I should add something?
The corrected patch:
diff -rup /home/lukasz/OPOS/SOURCE/gimp-git-original/gimp/plug-ins/script-fu/scheme-wrapper.c gimp/plug-ins/script-fu/scheme-wrapper.c
--- /home/lukasz/OPOS/SOURCE/gimp-git-original/gimp/plug-ins/script-fu/scheme-wrapper.c 2010-08-24 17:49:15.000000000 +0200
+++ gimp/plug-ins/script-fu/scheme-wrapper.c 2010-08-31 13:49:59.000000000 +0200
@@ -298,6 +298,10 @@ ts_interpret_stdin (void)
scheme_load_file (&sc, stdin);
}
+int load_script_file(const char *fname) {
+ return scheme_file_push(&sc,fname);
+}
+
gint
ts_interpret_string (const gchar *expr)
{
diff -rup /home/lukasz/OPOS/SOURCE/gimp-git-original/gimp/plug-ins/script-fu/scheme-wrapper.h gimp/plug-ins/script-fu/scheme-wrapper.h
--- /home/lukasz/OPOS/SOURCE/gimp-git-original/gimp/plug-ins/script-fu/scheme-wrapper.h 2010-08-24 17:49:15.000000000 +0200
+++ gimp/plug-ins/script-fu/scheme-wrapper.h 2010-08-31 13:50:17.000000000 +0200
@@ -32,6 +32,8 @@ const gchar * ts_get_success_msg (v
void ts_interpret_stdin (void);
+int load_script_file(const char *fname);
+
/* if the return value is 0, success. error otherwise. */
gint ts_interpret_string (const gchar *expr);
diff -rup /home/lukasz/OPOS/SOURCE/gimp-git-original/gimp/plug-ins/script-fu/script-fu-scripts.c gimp/plug-ins/script-fu/script-fu-scripts.c
--- /home/lukasz/OPOS/SOURCE/gimp-git-original/gimp/plug-ins/script-fu/script-fu-scripts.c 2010-08-24 17:49:15.000000000 +0200
+++ gimp/plug-ins/script-fu/script-fu-scripts.c 2010-08-31 13:51:33.000000000 +0200
@@ -602,13 +602,12 @@ script_fu_load_script (const GimpDatafil
command = g_strdup_printf ("(load \"%s\")", escaped);
g_free (escaped);
- if (! script_fu_run_command (command, &error))
+ if (! load_script_file(file_data->filename))
{
gchar *display_name = g_filename_display_name (file_data->filename);
gchar *message = g_strdup_printf (_("Error while loading %s:"),
display_name);
-
- g_message ("%s\n\n%s", message, error->message);
+ g_message ("%s", message);
g_clear_error (&error);
g_free (message);
@@ -625,6 +624,7 @@ script_fu_load_script (const GimpDatafil
g_free (command);
}
}
+/* end of modifications */
/*
* The following function is a GTraverseFunction.
diff -rup /home/lukasz/OPOS/SOURCE/gimp-git-original/gimp/plug-ins/script-fu/tinyscheme/scheme.c gimp/plug-ins/script-fu/tinyscheme/scheme.c
--- /home/lukasz/OPOS/SOURCE/gimp-git-original/gimp/plug-ins/script-fu/tinyscheme/scheme.c 2010-08-24 17:49:15.000000000 +0200
+++ gimp/plug-ins/script-fu/tinyscheme/scheme.c 2010-08-31 13:52:39.000000000 +0200
@@ -1415,6 +1415,12 @@ static void finalize_cell(scheme *sc, po
/* ========== Routines for Reading ========== */
+int scheme_file_push(scheme *sc, const char *fname) {
+ int ret = file_push(sc, fname);
+ file_pop(sc);
+ return ret;
+}
+
static int file_push(scheme *sc, const char *fname) {
FILE *fin = NULL;
if (sc->file_i == MAXFIL-1)
diff -rup /home/lukasz/OPOS/SOURCE/gimp-git-original/gimp/plug-ins/script-fu/tinyscheme/scheme.h gimp/plug-ins/script-fu/tinyscheme/scheme.h
--- /home/lukasz/OPOS/SOURCE/gimp-git-original/gimp/plug-ins/script-fu/tinyscheme/scheme.h 2010-08-24 17:49:15.000000000 +0200
+++ gimp/plug-ins/script-fu/tinyscheme/scheme.h 2010-08-31 13:53:08.000000000 +0200
@@ -152,6 +152,9 @@ SCHEME_EXPORT pointer scheme_eval(scheme
void scheme_set_external_data(scheme *sc, void *p);
SCHEME_EXPORT void scheme_define(scheme *sc, pointer env, pointer symbol, pointer value);
+SCHEME_EXPORT int scheme_file_push(scheme *sc, const char *fname);
+
+
typedef pointer (*foreign_func)(scheme *, pointer);
pointer _cons(scheme *sc, pointer a, pointer b, int immutable);
Best wishes,
Łukasz Czerwiński
_______________________________________________ Gimp-developer mailing list Gimp-developer@xxxxxxxxxxxxxxxxxxxxxx https://lists.XCF.Berkeley.EDU/mailman/listinfo/gimp-developer
