2004-11-13 Havoc Pennington <hp@redhat.com>

* test/glib/test-profile.c: fix this thing up a bit

	* dbus/dbus-message.c (dbus_message_new_empty_header): increase
	preallocation sizes by a fair bit; not sure if this will be an
	overall performance win or not, but it does reduce reallocs.

	* dbus/dbus-string.c (set_length, reallocate_for_length): ignore
	the test hack that forced constant realloc if asserts are
	disabled, so we can profile sanely. Sprinkle in some
	_DBUS_UNLIKELY() which are probably pointless, but before I
	noticed the real performance problem I put them in.
	(_dbus_string_validate_utf8): micro-optimize this thing a little
	bit, though callgrind says it didn't help; then special-case
	ascii, which did help a lot; then be sure we detect nul bytes as
	invalid, which is a bugfix.
	(align_length_then_lengthen): add some more _DBUS_UNLIKELY
	superstition; use memset to nul the padding instead of a manual
	loop.
	(_dbus_string_get_length): inline this as a
	macro; it showed up in the profile because it's used for loop
	tests and so forth
This commit is contained in:
Havoc Pennington 2004-11-13 07:07:47 +00:00
parent aa1294405e
commit 1b1dfafc34
6 changed files with 192 additions and 73 deletions

View file

@ -1,3 +1,27 @@
2004-11-13 Havoc Pennington <hp@redhat.com>
* test/glib/test-profile.c: fix this thing up a bit
* dbus/dbus-message.c (dbus_message_new_empty_header): increase
preallocation sizes by a fair bit; not sure if this will be an
overall performance win or not, but it does reduce reallocs.
* dbus/dbus-string.c (set_length, reallocate_for_length): ignore
the test hack that forced constant realloc if asserts are
disabled, so we can profile sanely. Sprinkle in some
_DBUS_UNLIKELY() which are probably pointless, but before I
noticed the real performance problem I put them in.
(_dbus_string_validate_utf8): micro-optimize this thing a little
bit, though callgrind says it didn't help; then special-case
ascii, which did help a lot; then be sure we detect nul bytes as
invalid, which is a bugfix.
(align_length_then_lengthen): add some more _DBUS_UNLIKELY
superstition; use memset to nul the padding instead of a manual
loop.
(_dbus_string_get_length): inline this as a
macro; it showed up in the profile because it's used for loop
tests and so forth
2004-11-10 Colin Walters <walters@verbum.org>
* dbus/dbus-spawn.c (check_babysit_events): Handle EINTR,

View file

@ -493,11 +493,13 @@ get_type_alignment (int type)
case DBUS_TYPE_ARRAY:
_dbus_assert_not_reached ("passed an ARRAY type to get_type_alignment()");
alignment = 0; /* quiet gcc */
break;
case DBUS_TYPE_INVALID:
default:
_dbus_assert_not_reached ("passed an invalid or unknown type to get_type_alignment()");
alignment = 0; /* quiet gcc */
break;
}
@ -1355,7 +1357,7 @@ dbus_message_new_empty_header (void)
++i;
}
if (!_dbus_string_init_preallocated (&message->header, 64))
if (!_dbus_string_init_preallocated (&message->header, 256))
{
dbus_free (message);
return NULL;

View file

@ -380,20 +380,28 @@ reallocate_for_length (DBusRealString *real,
new_allocated = real->allocated * 2;
/* if you change the code just above here, run the tests without
* the following before you commit
* the following assert-only hack before you commit
*/
/* This is keyed off asserts in addition to tests so when you
* disable asserts to profile, you don't get this destroyer
* of profiles.
*/
#ifdef DBUS_DISABLE_ASSERT
#else
#ifdef DBUS_BUILD_TESTS
new_allocated = 0; /* ensure a realloc every time so that we go
* through all malloc failure codepaths
*/
#endif
#endif /* DBUS_BUILD_TESTS */
#endif /* !DBUS_DISABLE_ASSERT */
/* But be sure we always alloc at least space for the new length */
new_allocated = MAX (new_allocated, new_length + ALLOCATION_PADDING);
new_allocated = MAX (new_allocated,
new_length + ALLOCATION_PADDING);
_dbus_assert (new_allocated >= real->allocated); /* code relies on this */
new_str = dbus_realloc (real->str - real->align_offset, new_allocated);
if (new_str == NULL)
if (_DBUS_UNLIKELY (new_str == NULL))
return FALSE;
real->str = new_str + real->align_offset;
@ -410,15 +418,15 @@ set_length (DBusRealString *real,
/* Note, we are setting the length not including nul termination */
/* exceeding max length is the same as failure to allocate memory */
if (new_length > real->max_length)
if (_DBUS_UNLIKELY (new_length > real->max_length))
return FALSE;
else if (new_length > (real->allocated - ALLOCATION_PADDING) &&
!reallocate_for_length (real, new_length))
_DBUS_UNLIKELY (!reallocate_for_length (real, new_length)))
return FALSE;
else
{
real->len = new_length;
real->str[real->len] = '\0';
real->str[new_length] = '\0';
return TRUE;
}
}
@ -780,6 +788,8 @@ _dbus_string_copy_to_buffer (const DBusString *str,
buffer[avail_len-1] = '\0';
}
/* Only have the function if we don't have the macro */
#ifndef _dbus_string_get_length
/**
* Gets the length of a string (not including nul termination).
*
@ -792,6 +802,7 @@ _dbus_string_get_length (const DBusString *str)
return real->len;
}
#endif /* !_dbus_string_get_length */
/**
* Makes a string longer by the given number of bytes. Checks whether
@ -812,7 +823,7 @@ _dbus_string_lengthen (DBusString *str,
DBUS_STRING_PREAMBLE (str);
_dbus_assert (additional_length >= 0);
if (additional_length > real->max_length - real->len)
if (_DBUS_UNLIKELY (additional_length > real->max_length - real->len))
return FALSE; /* would overflow */
return set_length (real,
@ -869,7 +880,7 @@ align_length_then_lengthen (DBusString *str,
_dbus_assert (alignment <= 8); /* it has to be a bug if > 8 */
new_len = _DBUS_ALIGN_VALUE (real->len, alignment);
if (new_len > (unsigned long) real->max_length - then_lengthen_by)
if (_DBUS_UNLIKELY (new_len > (unsigned long) real->max_length - then_lengthen_by))
return FALSE;
new_len += then_lengthen_by;
@ -879,21 +890,17 @@ align_length_then_lengthen (DBusString *str,
if (delta == 0)
return TRUE;
if (!set_length (real, new_len))
if (_DBUS_UNLIKELY (!set_length (real, new_len)))
return FALSE;
/* delta == padding + then_lengthen_by
* new_len == old_len + padding + then_lengthen_by
* nul the padding if we had to add any padding
*/
if (then_lengthen_by < delta)
{
unsigned int i;
i = new_len - delta;
while (i < (new_len - then_lengthen_by))
{
real->str[i] = '\0';
++i;
}
memset (&real->str[new_len - delta], '\0',
delta - then_lengthen_by);
}
return TRUE;
@ -1509,8 +1516,11 @@ _dbus_string_replace_len (const DBusString *source,
Len = 6; \
Mask = 0x01; \
} \
else \
Len = -1;
else \
{ \
Len = 0; \
Mask = 0; \
}
/**
* computes length of a unicode character in UTF-8
@ -1590,7 +1600,7 @@ _dbus_string_get_unichar (const DBusString *str,
c = *p;
UTF8_COMPUTE (c, mask, len);
if (len == -1)
if (len == 0)
return;
UTF8_GET (result, p, i, mask, len);
@ -2431,29 +2441,51 @@ _dbus_string_validate_utf8 (const DBusString *str,
while (p < end)
{
int i, mask = 0, char_len;
int i, mask, char_len;
dbus_unichar_t result;
unsigned char c = (unsigned char) *p;
const unsigned char c = (unsigned char) *p;
if (c == 0) /* nul bytes not OK */
break;
/* Special-case ASCII; this makes us go a lot faster in
* D-BUS profiles where we are typically validating
* function names and such. We have to know that
* all following checks will pass for ASCII though,
* comments follow ...
*/
if (c < 128)
{
++p;
continue;
}
UTF8_COMPUTE (c, mask, char_len);
if (_DBUS_UNLIKELY (char_len == -1))
if (_DBUS_UNLIKELY (char_len == 0)) /* ASCII: char_len == 1 */
break;
/* check that the expected number of bytes exists in the remaining length */
if (_DBUS_UNLIKELY ((end - p) < char_len))
if (_DBUS_UNLIKELY ((end - p) < char_len)) /* ASCII: p < end and char_len == 1 */
break;
UTF8_GET (result, p, i, mask, char_len);
if (_DBUS_UNLIKELY (UTF8_LENGTH (result) != char_len)) /* Check for overlong UTF-8 */
/* Check for overlong UTF-8 */
if (_DBUS_UNLIKELY (UTF8_LENGTH (result) != char_len)) /* ASCII: UTF8_LENGTH == 1 */
break;
#if 0
/* The UNICODE_VALID check below will catch this */
if (_DBUS_UNLIKELY (result == (dbus_unichar_t)-1)) /* ASCII: result = ascii value */
break;
#endif
if (_DBUS_UNLIKELY (!UNICODE_VALID (result))) /* ASCII: always valid */
break;
if (_DBUS_UNLIKELY (result == (dbus_unichar_t)-1))
break;
if (_DBUS_UNLIKELY (!UNICODE_VALID (result)))
break;
/* UNICODE_VALID should have caught it */
_dbus_assert (result != (dbus_unichar_t)-1);
p += char_len;
}

View file

@ -49,6 +49,13 @@ struct DBusString
unsigned int dummy8 : 3; /**< placeholder */
};
/* Unless we want to run all the assertions in the function,
* inline this thing, it shows up high in the profile
*/
#ifdef DBUS_DISABLE_ASSERT
#define _dbus_string_get_length(s) ((s)->dummy2)
#endif
dbus_bool_t _dbus_string_init (DBusString *str);
void _dbus_string_init_const (DBusString *str,
const char *value);
@ -91,7 +98,10 @@ dbus_bool_t _dbus_string_copy_data_len (const DBusString *str,
void _dbus_string_copy_to_buffer (const DBusString *str,
char *buffer,
int len);
#ifndef _dbus_string_get_length
int _dbus_string_get_length (const DBusString *str);
#endif /* !_dbus_string_get_length */
dbus_bool_t _dbus_string_lengthen (DBusString *str,
int additional_length);
void _dbus_string_shorten (DBusString *str,

View file

@ -327,7 +327,7 @@ remove_watch (DBusWatch *watch,
return; /* probably a not-enabled watch that was added */
watch_fd->removed = TRUE;
watch_fd->watch = NULL;
watch_fd->watch = NULL;
dbus_source->watch_fds = g_list_remove (dbus_source->watch_fds, watch_fd);
@ -409,7 +409,7 @@ timeout_toggled (DBusTimeout *timeout,
static void
free_source (GSource *source)
{
{
g_source_destroy (source);
}

View file

@ -21,18 +21,13 @@
*
*/
/* FIXME this test is wacky since both client and server keep
* sending each other method calls, but nobody sends
* a DBUS_MESSAGE_TYPE_METHOD_RETURN
*/
#include <config.h>
#include <glib.h>
#include <dbus/dbus-glib-lowlevel.h>
#include <stdlib.h>
#define N_CLIENT_THREADS 1
#define N_ITERATIONS 1000
#define N_ITERATIONS 4000
#define PAYLOAD_SIZE 30
#define ECHO_PATH "/org/freedesktop/EchoTest"
#define ECHO_INTERFACE "org.freedesktop.EchoTest"
@ -41,8 +36,21 @@
static const char *address;
static unsigned char *payload;
typedef struct
{
int iterations;
GMainLoop *loop;
} ClientData;
typedef struct
{
int handled;
GMainLoop *loop;
int n_clients;
} ServerData;
static void
send_echo_message (DBusConnection *connection)
send_echo_method_call (DBusConnection *connection)
{
DBusMessage *message;
@ -51,7 +59,7 @@ send_echo_message (DBusConnection *connection)
dbus_message_append_args (message,
DBUS_TYPE_STRING, "Hello World!",
DBUS_TYPE_INT32, 123456,
#if 1
#if 0
DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE,
payload, PAYLOAD_SIZE,
#endif
@ -62,12 +70,25 @@ send_echo_message (DBusConnection *connection)
dbus_connection_flush (connection);
}
static void
send_echo_method_return (DBusConnection *connection,
DBusMessage *call_message)
{
DBusMessage *message;
message = dbus_message_new_method_return (call_message);
dbus_connection_send (connection, message, NULL);
dbus_message_unref (message);
dbus_connection_flush (connection);
}
static DBusHandlerResult
client_filter (DBusConnection *connection,
DBusMessage *message,
void *user_data)
{
int *iterations = user_data;
ClientData *cd = user_data;
if (dbus_message_is_signal (message,
DBUS_INTERFACE_ORG_FREEDESKTOP_LOCAL,
@ -76,16 +97,15 @@ client_filter (DBusConnection *connection,
g_printerr ("Client thread disconnected\n");
exit (1);
}
else if (dbus_message_is_method_call (message,
ECHO_INTERFACE, ECHO_METHOD))
else if (dbus_message_get_type (message) == DBUS_MESSAGE_TYPE_METHOD_RETURN)
{
*iterations += 1;
if (*iterations >= N_ITERATIONS)
cd->iterations += 1;
if (cd->iterations >= N_ITERATIONS)
{
g_print ("Completed %d iterations\n", N_ITERATIONS);
exit (0);
g_main_loop_quit (cd->loop);
}
send_echo_message (connection);
send_echo_method_call (connection);
return DBUS_HANDLER_RESULT_HANDLED;
}
@ -97,9 +117,8 @@ thread_func (void *data)
{
DBusError error;
GMainContext *context;
GMainLoop *loop;
DBusConnection *connection;
int iterations;
ClientData cd;
g_printerr ("Starting client thread\n");
@ -112,28 +131,31 @@ thread_func (void *data)
exit (1);
}
iterations = 1;
context = g_main_context_new ();
cd.iterations = 1;
cd.loop = g_main_loop_new (context, FALSE);
if (!dbus_connection_add_filter (connection,
client_filter, &iterations, NULL))
client_filter, &cd, NULL))
g_error ("no memory");
context = g_main_context_new ();
loop = g_main_loop_new (context, FALSE);
dbus_connection_setup_with_g_main (connection, context);
g_printerr ("Client thread sending message to prime pingpong\n");
send_echo_message (connection);
send_echo_method_call (connection);
g_printerr ("Client thread sent message\n");
g_printerr ("Client thread entering main loop\n");
g_main_loop_run (loop);
g_main_loop_run (cd.loop);
g_printerr ("Client thread exiting main loop\n");
g_main_loop_unref (loop);
g_main_context_unref (context);
dbus_connection_disconnect (connection);
g_main_loop_unref (cd.loop);
g_main_context_unref (context);
return NULL;
}
@ -142,18 +164,23 @@ server_filter (DBusConnection *connection,
DBusMessage *message,
void *user_data)
{
ServerData *sd = user_data;
if (dbus_message_is_signal (message,
DBUS_INTERFACE_ORG_FREEDESKTOP_LOCAL,
"Disconnected"))
{
g_printerr ("Server thread disconnected\n");
exit (1);
g_printerr ("Client disconnected from server\n");
sd->n_clients -= 1;
if (sd->n_clients == 0)
g_main_loop_quit (sd->loop);
}
else if (dbus_message_is_method_call (message,
ECHO_INTERFACE,
ECHO_METHOD))
{
send_echo_message (connection);
sd->handled += 1;
send_echo_method_return (connection, message);
return DBUS_HANDLER_RESULT_HANDLED;
}
@ -164,14 +191,17 @@ static void
new_connection_callback (DBusServer *server,
DBusConnection *new_connection,
void *user_data)
{
{
ServerData *sd = user_data;
dbus_connection_ref (new_connection);
dbus_connection_setup_with_g_main (new_connection, NULL);
if (!dbus_connection_add_filter (new_connection,
server_filter, NULL, NULL))
server_filter, sd, NULL))
g_error ("no memory");
sd->n_clients += 1;
/* FIXME we leak the handler */
}
@ -179,11 +209,13 @@ new_connection_callback (DBusServer *server,
int
main (int argc, char *argv[])
{
GMainLoop *loop;
DBusError error;
DBusServer *server;
GTimer *timer;
int i;
double secs;
ServerData sd;
g_thread_init (NULL);
dbus_g_thread_init ();
@ -197,14 +229,20 @@ main (int argc, char *argv[])
return 1;
}
#ifndef DBUS_DISABLE_ASSERT
g_printerr ("You should probably turn off assertions before you profile\n");
#endif
address = dbus_server_get_address (server);
payload = g_malloc (PAYLOAD_SIZE);
dbus_server_set_new_connection_function (server,
new_connection_callback,
NULL, NULL);
loop = g_main_loop_new (NULL, FALSE);
&sd, NULL);
sd.handled = 0;
sd.n_clients = 0;
sd.loop = g_main_loop_new (NULL, FALSE);
dbus_server_setup_with_g_main (server, NULL);
@ -213,14 +251,27 @@ main (int argc, char *argv[])
g_thread_create (thread_func, NULL, FALSE, NULL);
}
timer = g_timer_new ();
g_printerr ("Server thread entering main loop\n");
g_main_loop_run (loop);
g_main_loop_run (sd.loop);
g_printerr ("Server thread exiting main loop\n");
secs = g_timer_elapsed (timer, NULL);
g_timer_destroy (timer);
g_printerr ("%g seconds, %d round trips, %g seconds per pingpong\n",
secs, sd.handled, secs/sd.handled);
#ifndef DBUS_DISABLE_ASSERT
g_printerr ("You should probably --disable-asserts before you profile as they have noticeable overhead\n");
#endif
g_printerr ("The following g_warning is because we try to call g_source_remove_poll() after g_source_destroy() in dbus-gmain.c, I think we need to add a source free func that clears out the watch/timeout funcs\n");
dbus_server_unref (server);
g_main_loop_unref (loop);
g_main_loop_unref (sd.loop);
return 0;
}