Make set_ps_display faster and easier to use
От | David Rowley |
---|---|
Тема | Make set_ps_display faster and easier to use |
Дата | |
Msg-id | CAApHDvocBvvk-0gWNA2Gohe+sv9fMcv+fK_G+siBKJrgDG4O7g@mail.gmail.com обсуждение исходный текст |
Ответы |
Re: Make set_ps_display faster and easier to use
|
Список | pgsql-hackers |
While doing some benchmarking of some fast-to-execute queries, I see that set_ps_display() popping up on the profiles. Looking a little deeper, there are some inefficiencies in there that we could fix. For example, the following is pretty poor: strlcpy(ps_buffer + ps_buffer_fixed_size, activity, ps_buffer_size - ps_buffer_fixed_size); ps_buffer_cur_len = strlen(ps_buffer); We already know the strlen of the fixed-sized part, so why bother doing strlen on the entire thing? Also, if we did just do strlen(activity), we could just memcpy, which would be much faster than strlcpy's byte-at-a-time method of copying. Adjusting that lead me to notice that we often just pass string constants to set_ps_display(), so we already know the strlen for this at compile time. So maybe we can just have set_ps_display_with_len() and then make a static inline wrapper that does strlen() so that when the compiler can figure out the length, it just hard codes it. After doing that, I went over all usages of set_ps_display() to see if any of those call sites knew the length already in a way that the compiler wouldn't be able to deduce. There were a few cases to adjust when setting the process title to contain the command tag. After fixing up the set_ps_display()s to use set_ps_display_with_len() where possible, I discovered some not so nice code which appends " waiting" onto the process title. Basically, there's a bunch of code that looks like this: const char *old_status; int len; old_status = get_ps_display(&len); new_status = (char *) palloc(len + 8 + 1); memcpy(new_status, old_status, len); strcpy(new_status + len, " waiting"); set_ps_display(new_status); new_status[len] = '\0'; /* truncate off " waiting" */ Seeing that made me wonder if we shouldn't just have something more generic for setting a suffix on the process title. I came up with set_ps_display_suffix() and set_ps_display_remove_suffix(). The above code can just become: set_ps_display_suffix("waiting"); then to remove the "waiting" suffix, just: set_ps_display_remove_suffix(); I considered adding a format version to append the suffix as there's one case that could make use of it, but in the end, decided it might be overkill, so I left that code like: char buffer[32]; sprintf(buffer, "waiting for %X/%X", LSN_FORMAT_ARGS(lsn)); set_ps_display_suffix(buffer); I don't think that's terrible enough to warrant making a va_args version of set_ps_display_suffix(), especially for just 1 instance of it. I also resisted making set_ps_display_suffix_with_len(). The new code should be quite a bit faster already without troubling over that additional function. I've attached the patch. David
Вложения
В списке pgsql-hackers по дате отправления: