Re: Move PinBuffer and UnpinBuffer to atomics

Поиск
Список
Период
Сортировка
От Alexander Korotkov
Тема Re: Move PinBuffer and UnpinBuffer to atomics
Дата
Msg-id CAPpHfdv5RxmJ1Z5Xz+mZu++=zTN_f9UCqN+Y4ZTGzg58835zmg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Move PinBuffer and UnpinBuffer to atomics  (Amit Kapila <amit.kapila16@gmail.com>)
Ответы Re: Move PinBuffer and UnpinBuffer to atomics  (Andres Freund <andres@anarazel.de>)
Список pgsql-hackers
Hi!

On Thu, Mar 31, 2016 at 4:59 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Tue, Mar 29, 2016 at 10:52 PM, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
Hi, Andres!

Please, find next revision of patch in attachment.


Couple of minor comments:

+  * The following two macroses

is macroses right word to be used here?

+  * of this loop.  It should be used as fullowing:

/fullowing/following

+  * For local buffers usage of these macros shouldn't be used.

isn't it better to write it as 

For local buffers, these macros shouldn't be used.


  static int ts_ckpt_progress_comparator(Datum a, Datum b, void *arg);
  

Spurious line deletion.

All of above is fixed.

+  * Since buffers are pinned/unpinned very frequently, this functions tries
+  * to pin buffer as cheap as possible.

/this functions tries

which functions are you referring here? Comment seems to be slightly unclear.

I meant just PinBuffer() there.  UnpinBuffer() has another comment in the body.  Fixed.
 
! if (XLogHintBitIsNeeded() && (pg_atomic_read_u32(&bufHdr->state) & BM_PERMANENT))

Is there a reason that you have kept macro's to read refcount and usagecount, but not for flags?
 
We could change dealing with flags to GET/SET macros.  But I guess such change would make review more complicated, because it would touch some places which are unchanged for now.  I think it could be done in a separate refactoring patch.

Apart from this, I have verified that patch compiles on Windows and passed regressions (make check)!

Thank you!  I didn't manage to try this on Windows.
 
Nice work!

Thank you!

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company 
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Joe Conway
Дата:
Сообщение: Re: So, can we stop supporting Windows native now?
Следующее
От: Emre Hasegeli
Дата:
Сообщение: Re: [PATCH] we have added support for box type in SP-GiST index