Comments 44
del
Использовать WeakReference для хранения кэшируемых объектов.
То есть использовать Dictionary<int, WeakReference<byte[]>>? Но ведь сами элементы словаря из никуда не денутся. Если словарь не чистить принудительно от мёртвых ссылок, то он тоже будет разрастаться.
Например, в моём случае ключ был довольно сложным: длинная строчка (query fragment от URI-запроса), без удаления ненужных элементов словарь разрастался до миллионов элементов и отъедал нехило так памяти.
При этом делать периодическую чистку было накладно, т.к. внезапно начать проверять миллион ссылок во время обработки запроса — это серьёзный лаг в обработке запросов. Вместо этого я написал свой велосипед WeakReferenceDictionary: словарь + связанный список. При каждом обращении к словарю я делал одну итерацию чистки: брал первый элемент из списка и, если он был жив, то отправлял его в конец, а если мёртв, то удалялся ключ из словаря.
Т.е. вы написали свой велосипед для кэширования. Не проще ли было использовать стандартный MemoryCache из библиотеки, который умеет ресайз и экспайрить элементы (например по размеру кэша или по таймауту), но при этом хранить в нем WeakRef вместо обычных ссылок. В таком случае нужна будет всего лишь одна доп. проверка при получении ссылки, чтобы убедиться что она не протухла.
Т.е. вы написали свой велосипед для кэширования.
Именно так. Велосипед получился простой и компактный: 81 строка, и это вместе с пустыми строчками и скобками.
Не проще ли было использовать стандартный MemoryCache из библиотеки, который умеет ресайз и экспайрить элементы
Нет, не проще. Мне не нужен expiration ни по времени, ни по занимаемой памяти, а только по факту существования ссылки на кэшируемый объект. Стандартный MemoryCache в это не умеет, надо все равно реализовывать CacheEntryChangeMonitor и делать проверку ручками.
Так что нет. Лучше написать простой велосипед, чем пытаться адаптировать под себя монстра.
А что со скоростью и потокобезопасностью вашего велосипеда? Он покрыт тестами? Вы уверены что в вашей реализации нет утечек, собственно? Что будет если оно будет жить месяц без рестарта? А если в него закинуть 100 ГБ данных? А если туда приедет 10 миллиардов уникальных элементов?
Я не сомневаюсь что вы реализовали простое и рабочее решение. И абсолютно четко понимаю, что полно случаев, когда это оправданно. Просто зачастую в существующих реализациях зачастую продуманы/протестированы очень многие edge cases, которые можно (осознанно либо же нет) пропустить в своей. Я всего лишь топлю за то, чтобы это был обоснованный выбор, а не просто потому что было лень разбираться с существующим решением либо просто очень хотелось свой родной величек выкатить в прод.
С этим всё в порядке, не беспокойтесь. Конечно, все решения полностью осознанные. У меня есть велосипеды и покруче.
А посмотреть можно?
Ну смотрите:
sealed class WeakReferenceDictionary<TKey, TValue>
where TKey : notnull
where TValue : class
{
readonly Dictionary<TKey, Entry> data = new();
readonly LinkedList<Entry> list = new();
public bool TryGetValue(TKey key, [MaybeNullWhen(returnValue: false)] out TValue value)
{
PerformCollectIteration();
if (!data.TryGetValue(key, out var entry))
{
value = default;
return false;
}
var obj = entry.Reference.Target;
if (obj != null)
{
value = (TValue)obj;
return true;
}
data.Remove(key);
list.Remove(entry);
value = default;
return false;
}
public void Add(TKey key, TValue value)
{
PerformCollectIteration();
var entry = new Entry(key, new WeakReference(value));
data.Add(key, entry);
entry.Node = list.AddLast(entry);
}
public void Remove(TKey key)
{
PerformCollectIteration();
if (data.TryGetValue(key, out var entry))
{
data.Remove(key);
list.Remove(entry.Node!);
}
}
void PerformCollectIteration()
{
var entry = list.First?.Value;
if (entry == null)
return;
list.RemoveFirst();
if (entry.Reference.IsAlive)
{
entry.Node = list.AddLast(entry);
}
else
{
data.Remove(entry.Key);
}
}
sealed class Entry
{
public readonly TKey Key;
public readonly WeakReference Reference;
public LinkedListNode<Entry>? Node;
public Entry(TKey key, WeakReference reference)
{
Key = key;
Reference = reference;
}
}
}Потокобезопасность — как у Dictionary.
Ну т.е. ее нет. Плюс к этому удвоенные требования по памяти... Очень "разумно" для кеша. Как раз то, о чем я и говорил.
Ну т.е. ее нет
Потому что для моей задачи это не нужно (блокировка используется во внешнем коде). Смысл переусложнять код?
Плюс к этому удвоенные требования по памяти…
Опять же пишете ерунду. В моём случае память уходит на хранение ключей и значений, которые могут быть весьма большими. От того, что ссылки на ключи и значения будут храниться дважды, потребление памяти вырастет процентов так на 5-10.
Зато для моей задачи критична стабильность отклика. Периодическое пробегание по всему словарю ради чистки — это очень плохой сценарий.
Конечно, в моей реализации тоже есть недостаток: худшее время вставки в Dictionary составляет O(N). Но это случается не так часто, как чистка.
Очень "разумно" для кеша. Как раз то, о чем я и говорил.
Спасибо, я вас понял. Вы почему-то топите исключительно за универсальные решения. Я же написал, почему MemoryCache меня не устраивает. У вас будут ещё советы? Или исключительно критика?
Ок, давайте по порядку.
Смысл переусложнять код?
Смысл в том, что в вашей реализации с внешним локом, любая операция (даже чтение) будет блокировать ВСЮ коллекцию. В случае же с ConcurrentDictionary<T> (который, собственно, и используется в дефолтной реализации MemoryCache) для вставки/удаления блокируется только один конкретный бакет, в котором в случае с хорошей хеш-функцией живет один единственный элемент. И даже для них в некоторых случаях используются атомарные операции вместо полноценного лока. Я уже молчу про операцию чтения, которая там вообще неблокирующая.
От того, что ссылки на ключи и значения будут храниться дважды, потребление памяти вырастет процентов так на 5-10.
Согласен, тут я некорректно выразился. Я скорее имел в виду число аллокаций. Что при интенсивной нагрузке и довольно долгом (по меркам GC) времени жизни объектов в кеше с большой долей вероятности приведет к их скоплению во 2 поколении и его фрагментации. Но тут я не могу утверждать на 100% - надо делать профилирование. Я уверен, что разработчики, имплементировавшие MemoryCache, делалали ее, причем для разных workload сценариев. Делали ли его вы?
Зато для моей задачи критична стабильность отклика.
Поэтому и не стоит зря аллоцировать лишнюю память. Благо GC сейчас очень многое прощает, не зря его полировали годами, но все же перенапрягать его не стоит.
Конечно, в моей реализации тоже есть недостаток
Да, вот он: list.Remove(entry);
Вот его сложность - O(n), в случае же с Dictionary (и Concurrent в том числе), в общем случае это O(1). И только лишь в случае полного вырождения O(n).
Но это случается не так часто, как чистка.
Чистка в MemoryCache производится в отдельном фоновом потоке, причем с определенным каденсом, а не при каждом чихе, так что никаких фризов быть не должно. Если вы конечно не под одноядерный/однопоточный чип пишете...
Вы почему-то топите исключительно за универсальные решения
Это не так, я топлю за проверенные решения. Выстраданные и полирующиеся годами, которыми, на данном этапе, учитывая зрелость фреймворка, осмелюсь предположить, являются очень многие базовые классы. Так же, многие из них оптимизированы с учетом нюансов работы CLR, коих ни вы, ни я, ни многие другие разработчики "со стороны" всех не знают.
Мой совет - подумайте как перенести блокировки внутрь реализации. Перепишите с использованием ConcurrentDictionary. Кстати, на Хабре есть очень годная статья про него https://habr.com/ru/company/skbkontur/blog/348508/. Еще для этого придется выкинуть LinkedList, но может оно и к лучшему, зачем он тут вообще, все равно вы каждый раз ищете entry в словаре? Проверять протухла ли WeakReference можно и при обращении к ключу. Если боитесть что stale records будут жрать память - добавте асинхронную проверку/очистку фоновом потоке. Только тогда получится что вы написали свою урощенную версию MemoryCache.
Чтобы сэкономить время, можете сходить по ссылке https://github.com/aspnet/Caching/blob/master/src/Microsoft.Extensions.Caching.Memory/MemoryCache.cs и посмотреть как оно все рализовано. Не знаю, можно ли переиспользовать код... Думаю да, с определенными оговорками - там вроде Apache 2.0
Или просто взять готовую реализацию MemoryCache.
Я же написал, почему
MemoryCacheменя не устраивает.Мне не нужен expiration ни по времени, ни по занимаемой памяти, а только по факту существования ссылки на кэшируемый объект.
Ваш критерий это и есть "по занимаемой памяти", только в случае с MemoryCache он сам заботится о том чтобы не распухнуть, и трекает время последнего обращения, чтобы очистить самые редко используемые записи, так что с ним вам и WeakReferences не особо будут нужны. Главное не создавать записи с опцией NeverRemove, и все будет хорошо.
И даже для них в некоторых случаях используются атомарные операции вместо полноценного лока. Я уже молчу про операцию чтения, которая там вообще неблокирующая.
Код с неблокирующими операциями будет намного более сложным. А оно мне надо? Преждевременные оптимизации — зло.
Атомарные операции вместо локов у меня используются, но только там, где это действительно необходимо.
Делали ли его вы?
Нет, в этом не было необходимости. Это код не является узким местом.
Так же, многие из них оптимизированы с учетом нюансов работы CLR, коих ни вы, ни я, ни многие другие разработчики "со стороны" всех не знают.
Я вот смотрю код и там не видно никаких оптимизаций с учётом нюансов работы CLR. Код как код, который писали самые обычные программисты.
Вот его сложность — O(n),
С чего вдруг? Это же LinkedList, а не List.
Если боитесть что stale records будут жрать память — добавте асинхронную проверку/очистку фоновом потоке. Только тогда получится что вы написали свою урощенную версию MemoryCache.
Ну вот, а у меня эта проверка сделана без фоновых потоков.
только в случае с MemoryCache он сам заботится о том чтобы не распухнуть, и трекает время последнего обращения, чтобы очистить самые редко используемые записи, так что с ним вам и WeakReferences не особо будут нужны.
А мне не нужно ни отслеживание последнего обращения, ни трекинга занимаемой памяти, потому что в кэше хранятся не копии, а ссылки на объекты, которые используются где-то ещё. То есть расход памяти в случае использования кэша — это хранение самого словаря, а это ничто по сравнению с самими объектами.
Дополнительный расход памяти появляется только в случае, если объект перестаёт использоваться, и ссылка на него остаётся исключительно в кэше. Поэтому и используется WeakReference, чтобы трекать такие объекты.
P.S. В целом, спасибо, навели на мысль, чтобы сравнить производительность велосипеда с библиотечным решением для типичных юзкейсов в проекте.
С чего вдруг? Это же LinkedList, а не List.
В вашей реализации TryGet используется именно он для удаления протухших ссылок.
Я не буду дальше с вами дискутировать, вижу мои доводы для вас не особо убедительны + я не в курсе всех особенностей вашей задачи.
Могу лишь предложить погонять ваше решение в условиях high memory pressure. Позамерять cache hit rate в различных сценариях. Собрать GC метрики при типичной нагрузке.
Да, действительно косяк. Должно быть entry.Node, как в другом месте.
Видимо, не вылезло, потому что количество одновременно живых объектов не очень большое, и в профилировщике это вообще никак не отображалось.
Upd: полез фиксить, оказалось, я вам старый код привёл из неактуальной ветки.
sealed class WeakReferenceDictionary<TKey, TValue>
where TKey : IEquatable<TKey>
where TValue : class
{
readonly Dictionary<TKey, Entry> data = new();
readonly Queue<Entry> list = new();
readonly object sync = new();
public bool TryGetValue(TKey key, [MaybeNullWhen(returnValue: false)] out TValue value)
{
lock (sync)
{
PerformCollectIteration();
if (!data.TryGetValue(key, out var entry))
{
value = default;
return false;
}
var obj = entry.Reference.Target;
if (obj != null)
{
value = (TValue) obj;
return true;
}
data.Remove(key);
entry.Removed = true;
value = default;
return false;
}
}
public void Add(TKey key, TValue value)
{
lock (sync)
{
PerformCollectIteration();
var entry = new Entry(key, new WeakReference(value));
data.Add(key, entry);
list.Enqueue(entry);
}
}
public void Remove(TKey key)
{
lock (sync)
{
if (data.TryGetValue(key, out var entry))
{
data.Remove(key);
entry.Removed = true;
}
PerformCollectIteration();
}
}
void PerformCollectIteration()
{
if (!list.TryDequeue(out var entry))
return;
if (entry.Removed)
return;
if (entry.Reference.IsAlive)
{
list.Enqueue(entry);
}
else
{
data.Remove(entry.Key);
}
}
sealed class Entry
{
public readonly TKey Key;
public readonly WeakReference Reference;
public bool Removed;
public Entry(TKey key, WeakReference reference)
{
Key = key;
Reference = reference;
}
}
}
И тут на сцену выходит эфемерон.
Проблема в том, что в C# его нет. Напрямую подписаться на финализацию объекта нельзя.
Есть решение в виде ConditionalWeakTable, к кэшируемому объекту прикрепить прокладку с финализатором, и в финализаторе реализовать логику удаления элемента из кэша.
Я этот вариант пробовал. Получалось дико неэффективно как по скорости, так и по памяти. Финализаторы в .NET — довольно тяжёлый механизм, и лучше его избегать.
Как раз с самым 1-м пунктом сам столкнулся, долго не мог понять,Ю почему у меня приложение само схлопывается (((. Помогли ребята на форумах.
На одном проекте у нас прям кладезь утечек был, на статью бы хватило. Большую часть уже не вспомню, но самые любопытные примеры запомнились.
Простой паттерн - отписаться от события, изменить коллекцию и подписаться обратно. Но если подписка/отписка сделана через анонимные методы, то отписка не делает ничего, а подписка - добавляет ещё один делегат. Пишу схематично, с телефона
x.OnChange -= () => DoSomething() ;
Uplate(x) ;
x.OnChange += () => DoSomething() ;Причины понятны, анонимный делегат каждый раз создаётся новый, поэтому отписка не происходит. Но пропустить такое довольно легко.
Window.ShowDialog() имеет задокументированную, но легко пропускаемую особенность: когда вызван без параметров, то родительским элементом для нового окна станет главное окно приложения. И когда в системе открывается большое количество диалоговых окон (в том числе, одно из другого, а из него ещё одно и т. д), со сложными моделями и со множеством взаимосвязей, то память кушается очень активно, а освобождается только при закрытии главного окна. Как решали, точно не помню - то ли явным образом вызывали диспоуз, то ли передавали параметр в ShowDialog, чтобы дочерние диалоги диспозилизь при закрытии родительского.
Ещё в системе были WPF элементы, встроенные в WinForm элементы (посредством ElementHost), а в паре мест было наоборот - WinForm-овский WebBrowser встраивался в WPF окно. И на WinForm всё привыкли, что диспоуз родителя диспозит всех детей, а на WPF всё привыкли, что Dispose не требуется, поэтому в ElementHost даже не предусмотрен диспоуз вложенной вьюшки, даже если она реализует IDisposable. Кто ж знал, что туда внутрь вкрутят браузер, который обязательно надо задиспоузить? Пришлось какие-то костыли прикручивать.
Вообще, конечно, яркий проект был. На thedailywft.com штук 8 статей по его мотивам.
Второй пример не плох, но может не решить проблем полностью.
Что значит "захватит локальную переменную" - как Я понимаю, компилятор создаст анонимный класс, в котором id станет полем, а лямбда с замыканием - методом.
Да, ссылка на "тяжелый" MyClass не останется висеть, но останется висеть ссылка на легкий класс с id, это тоже может быть неприятно.
Насколько я понимаю, проблема второго примера там в том, что в _jobQueue есть ссылка на экземпляр MyClass, что приводит к появлению цикличной ссылки и предотвращает утилизацию сборщиком мусора обоих объектов.
Если же мы будем использовать локальную переменную, которая в этом случае передается по значению, то сборщику мусора уже ничего не помешает удалить _jobQueue, так как здесь он уже не ссылается на экземпляр MyClass. При этом не важно, что именно представляет из себя делегат, который хранится в _jobQueue, главное, что он не ссылается на экземпляр MyClass.
Насколько я знаю, GC в .NET нет никакого дела до цикличных ссылок, он прекрасно их выпиливает.
Да, Вы правы. Получается, здесь дело не просто в циклической ссылке, а в замыкании, ссылка на которое будет сохранена в статическом поле.
Почему в статическом-то? Все, что сделает замыкание в данном случае — это создаст еще один метод в классе MyClass. Откуда статическое поле?
Тогда из-за чего утечка?
Так я ее в этом примере и не вижу. Это у автора статьи надо спрашивать.
Няп, дело в том, что _jobQueue не обязательно удаляется с удалением экземпляра MyClass. Ведь его при создании передают извне, так что это
вполне может быть какой-нибудь глобальный executor.
И что? Он держит ссылка на экземпляр работы, пока работа не будет выполнена, потом этот экземпляр отпустит. Это нормальное ожидаемое поведение, а не утечка.
Ну и с этим экземпляром работы захватит ещё и экземпляр MyClass, чего программист почему-то мог не ожидать. Я согласен, что пример не очень хороший, но утечка по такому принципу действительно может быть.
И что? Он держит ссылка на экземпляр работы, пока работа не будет выполнена, потом этот экземпляр отпустит. Это нормальное ожидаемое поведение, а не утечка.
Ну и с этим экземпляром работы захватит ещё и экземпляр MyClass, чего программист почему-то мог не ожидать. Я согласен, что пример не очень хороший, но утечка по такому принципу действительно может быть.
В том-то и дело, что это может быть ожидаемое поведение. Нельзя сходу называть это утечкой. Банальный вопрос: нас интересует значение на момент захвата или на момент выполнения?
Так что это не "не очень хороший", а весьма плохой пример.
Предположу из-за того, что подписка живет в более длинном контексте (должна жить с кучей подписчиков), и так как она живет - живет ее подписчик, даже если у него нет потребителей больше.
А пример заменяет захват подписчика на захват замыкания (в котором может быть меньше данных.
А циклические ссылки GC умеет дропать, если они между мертвыми объектами.
Кстати, сейчас проверил генерируемый IL, и да, никакого static. Хотя встречал об этом не один раз на просторах Интернета.
Пример с таймером тоже плохой. Там будет точно наоборот, таймер будет собран сборщиком, когда этого не ожидаешь. Это если билдить в релиз, если в дебаг, то все как вы сказали.
Разве? В чем тогда смысл таймера, если он останавливается сам, когда ему вздумается?
С чего вдруг? Посмотрите исходники:
https://github.com/dotnet/corert/blob/master/src/System.Private.CoreLib/shared/System/Threading/Timer.cs
Пока таймер может тикать, он сидит в TimerQueue.
As long as you are using a Timer, you must keep a reference to it. As with any managed object, a Timer is subject to garbage collection when there are no references to it. The fact that a Timer is still active does not prevent it from being collected.
https://docs.microsoft.com/en-us/dotnet/api/system.threading.timer?view=net-5.0#remarks
Update:
Этот код ведет себя поразному в .Net Framework и .Net Core:
using System;
using System.Threading;
namespace ConsoleApp1
{
class Program
{
static void Main(string[] args)
{
var timer = new Timer(OnTimer, null, 0, 1000);
Console.ReadLine();
}
static void OnTimer(object state)
{
Console.WriteLine(DateTime.Now.TimeOfDay);
GC.Collect();
}
}
}Если билдить в Release, то в .Net Framework таймер сработает лишь 1 раз, а в .Net Core будет продолжать работать пока не остановишь.
Забыли загрузку сборок и их динамическую генерацию. В моем опыте был случай, когда память текла из-за создания экземпляров типа Regex с опцией Compiled. С этой опцией создается динамическая сборка, которая никогда не выгружается.
Кстати, в первом пункте никакой утечки нет. Здесь экземпляр класса будет жить до тех пор, пока жив объект, на событие которого он подписан. Как только сборщик мусора доберётся до него, ваш экземпляр тоже удалится. Вот от статических событий нужно обязательно отписываться, это правда.
Самое интересное, что выше есть комментарий от человека, который сам с этой утечкой сталкивался.
Она может быть, а может не быть. Согласен, что подписываться на событие объекта, временем жизни которого мы не управляем, не лучшая практика. Этот пункт как раз об этом: объект вставляет ссылку на себя в объект, который приходит извне, тем самым ставя себя в зависимость от времени жизни этого объекта. Если ваш объект транзиентный, а подписывается на синглтон, например, то будет утечка.
Но в общем случае это не обязательно утечка: если мы знаем время жизни объекта, на который мы подписываемся, и понимаем отношения этого объекта с подписчиком, то вполне можно не отписываться, и оставить это на откуп GC.
8 способов вызвать утечки памяти в .NET