Re: Performance Improvement by reducing WAL for Update Operation
От | Amit kapila |
---|---|
Тема | Re: Performance Improvement by reducing WAL for Update Operation |
Дата | |
Msg-id | 6C0B27F7206C9E4CA54AE035729E9C383BEA1C90@szxeml509-mbx обсуждение исходный текст |
Ответ на | Re: Performance Improvement by reducing WAL for Update Operation (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>) |
Ответы |
Re: Performance Improvement by reducing WAL for Update
Operation
|
Список | pgsql-hackers |
On Friday, December 07, 2012 2:28 PM Kyotaro HORIGUCHI wrote: > Hello, I looked into the patch and have some comments. Thank you for reviewing the patch. > From the restriction of the time for this rather big patch, > please excuse that these comments are on a part of it. Others > will follow in few days. It's perfectly fine. >==== heaptuple.c > >noncachegetattr(_with_len): > >- att_getlength should do strlen as worst case or VARSIZE_ANY > which is heavier than doing one comparizon, so I recommend to > add 'if (len)' as the restriction for doing this, and give NULL > as &len to nocachegetattr_with_len in nocachegetattr. Fixed. >heap_attr_get_length_and_check_equals: > >- Size seems to be used conventionary as the type for memory > object length, so it might be better using Size instead of > int32 as the type for *tup[12]_attr_len in parameter. Fixed. >- This function returns always false for attrnum <= 0 as whole > tuple or some system attrs comparison regardless of the real > result, which is a bit different from the anticipation which > the name gives. If you need to keep this optimization, it > should have the name more specific to the purpose. The heap_attr_get_length_and_check_equals function is similar to heap_tuple_attr_equals, the attrnum <= 0 check is required for heap_tuple_attr_equals. >haap_delta_encode: > >- Some misleading variable names (like match_not_found), > some reatitions of similiar codelets (att_align_pointer, pglz_out_tag), > misleading slight difference of the meanings of variables of > similar names(old_off and new_off and the similar pairs), > and bit tricky use of pglz_out_add and pglz_out_tag with length = 0. > > These are welcome to be modified for better readability. The variable names are modified, please check them once. The (att_align_pointer, pglz_out_tag) repetition code is added to take care of padding only incase of values are equal. Use of pglz_out_add and pglz_out_tag with length = 0 is done because of code readability. >==== heapam.c > >fastgetattr_with_len > >- Missing left paren in the line 867 ('nocachegetattr_with_len(tup)...') > >- Missing enclosing paren in heapam.c:879 (len, only on style) > >- Allowing len = NULL will be good for better performance, like > noncachegetattr. Fixed. except len=NULL because fastgetattr is modified as below comment. >fastgetattr > >- I suppose that the coding covension here is that macro and > alternative c-code are expected to be look similar. fastgetattr > looks quite differ to corresponding macro. Fixed. Another change is also done to handle the history size of 2 bytes which is possible with the usage of LZ macro's for deltaencoding. With Regards, Amit Kapila.
Вложения
В списке pgsql-hackers по дате отправления: