ALT Linux Team development discussions
 help / color / mirror / Atom feed
From: Vladislav Zavjalov <slazav@altlinux.org>
To: ALT Linux Team development discussions <devel@lists.altlinux.org>
Subject: Re: [devel] re APT patch: Use same [32bit] type for all offsets to dynamically allocated map
Date: Thu, 13 Feb 2020 05:19:51 +0300
Message-ID: <20200213021950.GA11942@imap.altlinux.org> (raw)
In-Reply-To: <alpine.LFD.2.20.2002130352160.6363@imap.altlinux.org>

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 потому, что где-то
могут вычисляться суммы/разности таких чисел и авторы боятся
переполнения в таких местах.


  reply	other threads:[~2020-02-13  2:19 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-13  1:13 Ivan Zakharyaschev
2020-02-13  2:19 ` Vladislav Zavjalov [this message]
2020-02-13  5:34 ` Ivan Zakharyaschev
2020-02-13 12:33   ` Aleksei Nikiforov
2020-02-13 14:05     ` Ivan Zakharyaschev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200213021950.GA11942@imap.altlinux.org \
    --to=slazav@altlinux.org \
    --cc=devel@lists.altlinux.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

ALT Linux Team development discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://lore.altlinux.org/devel/0 devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 devel devel/ http://lore.altlinux.org/devel \
		devel@altlinux.org devel@altlinux.ru devel@lists.altlinux.org devel@lists.altlinux.ru devel@linux.iplabs.ru mandrake-russian@linuxteam.iplabs.ru sisyphus@linuxteam.iplabs.ru
	public-inbox-index devel

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://lore.altlinux.org/org.altlinux.lists.devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git