From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 13 Feb 2020 05:19:51 +0300 From: Vladislav Zavjalov To: ALT Linux Team development discussions Message-ID: <20200213021950.GA11942@imap.altlinux.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [devel] re APT patch: Use same [32bit] type for all offsets to dynamically allocated map X-BeenThere: devel@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: ALT Linux Team development discussions List-Id: ALT Linux Team development discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Feb 2020 02:19:51 -0000 Archived-At: List-Archive: List-Post: On Thu, Feb 13, 2020 at 04:13:17AM +0300, Ivan Zakharyaschev wrote: > Со временем я замечаю больше тонкостей про memory management в APT, > например, обратил-таки когда-то внимание на следующее место в mmap.h: > > /* This should be a 32 bit type, larger tyes use too much ram and smaller > types are too small. Where ever possible 'unsigned long' should be used > instead of this internal type */ > typedef unsigned int map_ptrloc; > > Я пытался догадаться, что имели в виду авторы, и подумал, что когда > нам нужно хранить указатель в структуре, попадающей в область памяти, > отмапленную в файл ("кеш" информации о всех пакетах), то мы хотим > чтобы это всё занимало поменьше места и вместо указателя храним в > структуре "индекс" (offset) в массиве, соответствующем этой области, и > делаем его 32-битным (грубо говоря). (На LP64-платформах, таких как > x86_64, int 32битный.) > > При этом авторы призывают где возможно использовать unsigned long > (64-бита, как и указатели, на x86_64). "Где возможно" -- это, наверное, > до тех пор, пока мы индекс не записываем в структуру, которая будет > сохранена в этом мапе. Т.е. когда нам просто надо поработать с > данными. > > Я не очень понимаю, к каким местам кода в программе этот призыв > применим. > > Но всё же меня озадачивает, что следующий патч идёт как бы вразрез с этим > призывом -- он изживает unsigned long из всех методов класса DynamicMMap, > которые возвращают такие индексы. > > Хотелось бы понять: авторы APT хотели бессмысленного или у их задумки > был какой-то смысл и этот патч её нарушает? > > Связанный вопрос, о котором я задумался: в тех местах, где эти > "индексы" (offsets) вычисляются (чтобы их вернуть), какие гарантии, > что они не выйдут за пределы 32 бит? Возможно, с такими гарантиями > лучше в тех местах в коде, где эти значения окончательно попадают в > поля структур типа map_ptrloc, которые будут хранится в мапе ("кеше")? > > Что всё же лучше (и почему): оставить, как было, или перейти на 32 > бита в соответствии с этим патчем? > > Если перейти, мне кажется, их комментарий с призывом к unsigned long > хорошо бы тоже заменить на обоснование новой концепции (а то он будет > противоречить тому, что мы видим в коде, и сбивать с толку ещё > сильнее, хотя и раньше он был загадочным). > Из общих соображений кажется, что такую задачу надо решать, сделав get_* и set_* методы в этой структуре. Пусть они принимают int64_t (и снаружи пусть все работают с этим типом), а внутри (если хочется экономить место) пусть хранят private int32_t. И переполнение пусть проверяют. Снаружи могло быть полезно использовать int64_t потому, что где-то могут вычисляться суммы/разности таких чисел и авторы боятся переполнения в таких местах.