LINUX.ORG.RU

C++ жадина или я дурак? (проблема с delete)

 , ,


0

4

Случилось страшное, убил весь день и вечер, не смог разобраться сам. Пересел с Си на Плюсы. Написал для своих задач реализацию листа (ниже полный листинг), все компилится и робит, но жрет память, точнее не отдает ее ОС после удаления узла из списка, судя по таск менеджеру, а только лишь после выхода. В чем может быть дело, друзья, что я делаю не так?:)

namespace Brevity {

    template <class T> class List {

    public:

        List();
        ~List();
        void add(T data);
        bool remove(size_t ind);
        bool swap(size_t src, size_t dst);
        size_t count();
        T *get(size_t ind);

    private:

        class Node {

        public:
            Node(T data);
            T data;
            Node *prev;
            Node *next;

        };

        Node *head;
        size_t length;
        Node *getNode(size_t ind);

    };

    template <class T> List<T>::Node::Node(T data) {
        this->data = data;
        this->prev = NULL;
        this->next = NULL;
    }

    template <class T> List<T>::List() {
        this->length = 0;
        head = NULL;
    }

    template <class T> List<T>::~List() {
        while (this->length > 0) {
            this->remove(0);
        }
    }

    template <class T> void List<T>::add(T data) {
        List::Node *newNode = new List::Node(data);
        if (this->length == 0) {
            newNode->prev = newNode;
            newNode->next = newNode;
            this->head = newNode;
        } else {
            newNode->prev = this->head->prev;
            newNode->next = this->head;
            this->head->prev->next = newNode;
            this->head->prev = newNode;
        }
        this->length++;
    }

    template <class T> bool List<T>::remove(size_t ind) {
        List::Node *curNode = List::getNode(ind);
        if (curNode == NULL) {
            return false;
        } else {
            if (ind == 0) {
                this->head = curNode->next;
            }
            if (this->length > 1) {
                curNode->prev->next = curNode->next;
                curNode->next->prev = curNode->prev;
            }
            delete curNode;
            this->length--;
            return true;
        }
    }

    template <class T> bool List<T>::swap(size_t src, size_t dst) {
        List::Node *srcNode, *dstNode;
        srcNode = List::getNode(src);
        dstNode = List::getNode(dst);
        if (srcNode == NULL || dstNode == NULL) {
            return false;
        } else {
            T tmp;
            tmp = srcNode->data;
            srcNode->data = dstNode->data;
            dstNode->data = tmp;
            return true;
        }
    }

    template <class T> typename List<T>::Node* List<T>::getNode(size_t ind) {
        if (this->length <= ind) {
            return NULL;
        } else {
            List::Node *curNode;
            size_t curInd;
            int increment;
            if (ind <= (size_t) (this->length / 2)) {
                curNode = this->head;
                curInd = 0;
                increment = 1;
            } else {
                curNode = this->head->prev;
                curInd = this->length - 1;
                increment = -1;
            }
            while (curInd != ind) {
                curNode = (increment > 0) ? curNode->next : curNode->prev;
                curInd += increment;
            }
            return curNode;
        }
    }

    template <class T> size_t List<T>::count() {
        size_t count = this->length;
        return count;
    }

    template <class T> T *List<T>::get(size_t ind) {
        return &List::getNode(ind)->data;
    }

}

int main(){

    Brevity::List <size_t > myList;
    for (size_t i = 0; i < 10000000; ++i) {
        myList.add(i);
    }

    for (size_t i = 0; i < 10000000; ++i) {
        myList.remove(0);
    }

    char a;
    std::cin >> a;

    for (int i = 0; i < myList.count(); ++i) {
        std::cout << *myList.get((size_t) i) << " ";
    }

}


Последнее исправление: zasyadko (всего исправлений: 2)

Ответ на: комментарий от pftBest

предполагает наличие и поиска

Вы итератор поиском называете? А откуда вообще взялось «в случайное место»? Может быть вообще только и надо в ~середину втыкать? :)

Да если список так фрагментирован, что вечно не попадает в кеш при пробежке по указателям, то вставка/удаления данных если они были в векторе значить проходила в таких объёмах и с такими тормозами, что давно бы всех достала...

vodz ★★★★★
()
Ответ на: комментарий от invy

в примитивном бенчмарке вектор начинает довольно быстро сливать

Вот интереснее: https://baptiste-wicht.com/posts/2012/12/cpp-benchmark-vector-list-deque.html

Сначала сказал что Страуструп не прав, а ниже привел ссылку где почти на каждом графике list сливает вектору :)

pftBest ★★★★
()
Ответ на: комментарий от pftBest

Подозреваю что когда я мерял я сделал халтуру и померял вставку вначало :)

Не исключаю так же разницу в железе.

invy ★★★★★
()
Последнее исправление: invy (всего исправлений: 1)
Ответ на: комментарий от anonymous

Ты просто процитировал надпись на общеизвестном сайтишке

Сказал анон на никому не известном сайтишке. Это не SO цитирует ЛОР, это ЛОР цитирует SO.

стандарт цепепе, которым так кичатся цепепе писатели, нигде не предписывает

Стандарт также не говорит что оно должно быть отвязано от malloc, этим и пользуются все.

Прочитав тех же Страуструпа и Саттера, ты обнаружишь для себя, что эти известные авторы гайдлайнов и прочих «стандартов кодирования» также говорят о new/delete как о неких абстрактных операторах выделения/высвобождения оперативной памяти

Они не запрещают абстракциям юзать системные вызовы malloc/free. А если ты не знаешь на чем базируется абстракция на твоей системе - то нельзя говорить о понимании чего либо.

Случай описываемый на «общеизвестном сайтишке» покрывает glib*, CLang, Apache and MSFT. Это 99.999% всех систем на которых можно что-либо писать. Кукарекать о «а вот на распберри собранной с помощью паяльника из жопы и библиотеки github.com/xxxxx new базируется на HeapAlloc» глупо.

PPP328 ★★★★★
()
Ответ на: комментарий от PPP328

Стандарт также не говорит что оно должно быть отвязано от malloc, этим и пользуются все.

Смотрю, ты неумело изворачиваешься :-) Попытка вывернуть наизнанку официальный документ по цепепе, приводя в качестве доводов частные случаи, вещая о каких-то 99 процентах выглядит глупо и непрофессионально, мягко говоря :-)

Кукарекать о «а вот на распберри собранной с помощью паяльника из жопы и библиотеки github.com/xxxxx new базируется на HeapAlloc» глупо.

Это да, так и есть :-) Глупо :-) Ведь кроме тебя никто ни о какой распберри здесь не говорит :-) Тебе по стандарт, а ты в ответ про glib и распбери пи :-) Всё, как бы, ясно :-) Лол :-)

anonymous
()
Ответ на: комментарий от pftBest

Скомпилировал предоставленые по ссылке бенчмарки и прогнал у себя. Общее высказывание могу сделать такое: вектор начинает стабильно сливать если хранимые объекты больше 128 бит.

invy ★★★★★
()
Ответ на: комментарий от KennyMinigun

Оставлю это здесь: https://youtu.be/YQs6IC-vgmo

Я для себя давно составил мнение, что все эти элементарные структуры данных (вита вектора, списка, и т. п.) - это просто попытка теоретиков (в хорошем понимании этого слова) классифицировать структуры данных. В реальных задачах идут комбинации структур. Например, ты можешь сделать список массивов, что удобно при работе с данными, которые только частично могут подгружаться в память (например, огромный бинарный файл на диске). Или комбинацию массива и списка, которая позволит вставлять элементы так же быстро, как в списке, и иметь к ним доступ почти так же быстро как в массиве (с процессом оптимизации на фоне).

Kroz ★★★★★
()
Последнее исправление: Kroz (всего исправлений: 1)
Ответ на: комментарий от zasyadko

1. Для прямого ответа на твой вопрос есть madvise MADV_DONTNEED 2. Используй shared_ptr, unique_ptr 3. Используй stl::list

anonymous
()

Ну в общем то вопрос можно закрывать, та же ситуация и в Си наблюдается, просто это особенность фрагментации при выделении, освобождении памяти. Раньше не обращал на это внимания как то. По поводу советов использовать std прислушался, только переопределил некоторые методы, например получение размера и проверку на пустоту, уж больно она долгая в стандарте, ну и заверну потом в потокобезопасную обёртку. Всем участвовашим в обсуждении спасибо!

zasyadko
() автор топика
Ответ на: комментарий от zasyadko

Ты отнаследовал свой класс от std-контейнера? Если да, то заворачивай назад, в std не рекомендуется так делать, т.к. деструкторы невиртуальные.

ncuxer
()
Ответ на: комментарий от ncuxer

деструкторы невиртуальные.

Пока ты не добавляешь новых данных, на это похрену.

anonymous
()
Ответ на: комментарий от ncuxer

деструкторы невиртуальные.

Пока ты не добавляешь новых данных, на это похрену.

anonymous
()
Ответ на: комментарий от ncuxer

так как лучше? и главное почему? Вот например, стоит отнаследоваться?

template <class T>
    class Queue: private std::queue<T> {
    private:
        size_t length;
    public:
        Queue();
        size_t size();
        bool empty();
        void put(const T& value);
        T pull();
    };
или объвить внутри приватным членом и юзать внутри своего класса?
    template <class T>
    class Queue {
    private:
        std::queue<T> q;
        size_t length;
    public:
        Queue();
        size_t size();
        bool empty();
        void put(const T& value);
        T pull();
    };

zasyadko
() автор топика
Ответ на: комментарий от Kroz

Охренеть! Оказывается всё сложное собирается из простого! Гениальная мысль!

anonymous
()
Ответ на: комментарий от zasyadko

В первом случае если выполнить код

std::queue<int> *vec = new Queue<int>();
delete vec;

То все выполнится без ошибок и отработает тоже, но вот деструктор Queue не вызовется, т.к. деструктор невиртуальный, и будет утечка памяти.

Во втором случае этой проблемы нет, всё хорошо.

Сверху анонимус верно сказал, что в общем-то это не такая уж и проблема и эта ситуация не возникнет в твоём коде с 99% вероятностью. Но это явный smell code, как по мне, и не стоит того.

ncuxer
()
Ответ на: комментарий от zasyadko

Прошу прощения, не заметил, что в первом случае private наследование. С ним такой проблемы нет.

Но в общем случае разработчики стандарта не рекомендуют это делать, это я точно помню.

Погуглил, нашел хороший ответ на SO https://stackoverflow.com/questions/6806173/subclass-inherit-standard-contain...

ncuxer
()
Ответ на: комментарий от anonymous

С другой стороны, стандарт цепепе, которым так кичатся цепепе писатели, нигде не предписывает, что new/delete обязан быть реализован на основе malloc()/free()

21.6.2.1 Single-object forms [new.delete.single]

void* operator new(std::size_t size);
void* operator new(std::size_t size, std::align_val_t alignment);
4 Default behavior:
(4.1) — Executes a loop: Within the loop, the function first attempts to allocate the requested storage. Whether the attempt involves a call to the C standard library functions malloc or aligned_alloc is unspecified.

Короче, либо malloc, либо aligned_alloc. И то, и другое освобождается с помощью free

anonymous
()
Ответ на: комментарий от anonymous

Короче, либо malloc, либо aligned_alloc. И то, и другое освобождается с помощью free

С какой стати? :-)

Whether the attempt involves a call to the C standard library functions malloc or aligned_alloc is unspecified.

Да, я в курсе :-) Теперь переведи на русский язык и ещё раз подумай о чём тут говорится, а о чём тут не говорится :-) Лол :-)

anonymous
()
Ответ на: комментарий от anonymous

Whether the attempt involves a call to the C standard library functions malloc or aligned_alloc is unspecified.

Короче, либо malloc, либо aligned_alloc.

Хаха. :-) У тебя явно проблемы с английским. :-)

anonymous
()
Ответ на: комментарий от ncuxer

Спасибо за ответ, да и второй вариант даже смотрится удобнее с точки зрения написания кода, все более прозрачно:)

zasyadko
() автор топика
Ответ на: комментарий от anonymous

С какой стати? :-)

http://en.cppreference.com/w/c/memory/aligned_alloc

On success, returns the pointer to the beginning of newly allocated memory. The returned pointer must be deallocated with free() or realloc().

anonymous
()
Ответ на: комментарий от zasyadko

Спасибо за ответ, да и второй вариант даже смотрится удобнее с точки зрения написания кода, все более прозрачно:)

Никакой разницы между приватным наследованием и включением с т.з. размера объекта нет :-) Приватное наследование как-бы ужесточает интерфейс родителя в потомке, а не для выражения отношения «является» :-) Оно используется вместо включения в тех случаях, когда необходимо заместить виртуальные функции :-) Когда родительский класс не определяет виртуальный деструктор, то приватное наследование порождает те же проблемы, что и открытое наследование - если объект удаляется через указатель на родителя, то деструктор потомка не вызывается :-) Вот тебе простой пример:

#include <iostream>
#include <vector>

class Vecint : std::vector<int> {
public:
  Vecint() : std::vector<int>(){};
  ~Vecint() { std::cout << "~Vecint()" << std::endl; };

  static
  std::vector<int>* make_vector() { return new Vecint; };
};

int main()
{
  std::vector<int>* v = Vecint::make_vector();
  delete v; // oops, ~Vecint will be ignored!
}

anonymous
()
Ответ на: комментарий от anonymous

On success, returns the pointer to the beginning of newly allocated memory. The returned pointer must be deallocated with free() or realloc().

Понятно, что ты умеешь ретранслировать надписи из интернета :-) Но суть написанного в стандарте от этого не меняется :-)

anonymous
()
Ответ на: комментарий от pftBest

Читаю я вас, и мне смешно. Буквально несколько лет мне тут весь форум доказывал, что «список круче», а как страуструп(или кто там?) сказал «список говно» - так все вдруг прозрели. И те, кто до этого орал одно теперь орут уже другое. Ах да, я не про тебя - я не знаю делал ли то тоже самое, либо нет.

saifuluk
()
Ответ на: комментарий от pftBest

Разница в том, что элементы связного списка все находятся в разных местах в памяти, и когда ты ищешь место для вставки, на каждом переходе происходит cache miss и данные берутся из оперативки. А в векторе все элементы лежат рядом, и потому процессор может легко предсказать обращения к ним, и они все попадают в кеш.

Чистейшая муть. Где находятся элементы списка зависит от аллокатора и если он дефолтный, то как минимум от состояния хипа.

Никакие кэшмисы ни на что не влияют - это натягивание совы на глобус от людей, которые ничего не понимают ни в кешах, ни в мисах. Из кешмиса ничего не следует.

А то, что кто-то в своём втюне увидел кешмис - это лишь свойство процессора, который в силу своего устройства не умеет общаться с памятью.

Производительность линейного чтения/записи никак не связано с кешами. Кэш ей только мешает. И уж тем более не связано ни с какими «мисами».

по которому видно, что vector становится медленнее списка только тогда когда размер каждого элемента превышает 200 байт, и ему приходится действительно много данных копировать.

Манипуляция настолько глупая. Размер елемента не имеет никакого значения без указания длинны. В данном случае длинна 10к, что равняется 2метрам. При длине вектора в 2метра - он всегда будет находится в l3 - какая тут нахрен память?

Эти эксперты, такие эксперты.

saifuluk
()
Ответ на: комментарий от saifuluk

Смешно еще и потому, что необходимо рассматривать преимущества той или иной структуры данных относительно прикладной задачи, и не более того. Проведение голых тестов на крутость без объяснения контекста - несусветная чушь априори.

zasyadko
() автор топика
Ответ на: комментарий от saifuluk

Согласен, муть конечно, полезли зачем то в кэш..... А ведь истина то на поверхности, чтобы найти k-ый элемент списка требуется k переходов по ссылке в памяти, а в векторе требуется всего лишь вычислить смещение элемента, зная его размер.

zasyadko
() автор топика
Ответ на: комментарий от zasyadko

А ведь истина то на поверхности

Кто ж вам запрещает преобразовать список в вектор, если надо закончить добавление неизвестного количества элементов со вставками и удалением, для чего лучше всего подходит списки и перейти к обращениям по N-ным элементам, скажем для сортировки?

vodz ★★★★★
()
Ответ на: комментарий от zasyadko

ведь истина то на поверхности, чтобы найти k-ый элемент списка требуется k переходов по ссылке в памяти, а в векторе требуется всего лишь вычислить смещение элемента, зная его размер.

На самом деле для того чтобы тест получился честный, поиск элемента в векторе тоже делали линейным. Так что количество прочитанных элементов одинаково в обоих случаях.

pftBest ★★★★
()
Ответ на: комментарий от pftBest

На самом деле для того чтобы тест получился честный, поиск элемента в векторе тоже делали линейным. Так что количество прочитанных элементов одинаково в обоих случаях.

Позвольте полюбопытствовать откуда сведения? Где это в векторе для взятия элемента (поиска по индексу) все предыдущие перебирают? Или вы имели ввиду поиск по значению элемента?

zasyadko
() автор топика
Ответ на: комментарий от zasyadko

Это написано в статье из которой я привел свой график:

Let us test this by doing insertions of random values. We will keep the data structure sorted and to get to the right position we will use linear search for both vector and the linked-list. Of course this is silly way to use the vector but I want to show how effective the adjacent memory vector is comparedto the disjoint memory linked-list.

https://kjellkod.wordpress.com/2012/02/25/why-you-should-never-ever-ever-use-...

Конечно для вектора можно сделать бинарный поиск, соорудить кучу, или вообще отсортировать другим алгоритмом, но это было бы нечестное сравнение.

pftBest ★★★★
()
Последнее исправление: pftBest (всего исправлений: 1)
Ответ на: комментарий от pftBest

Лично я всегда отталкиваюсь от задачи. Пример 1. Допустим стоит задача о выборе структуры данных для организации очереди, определяем те операции которые хотим совершать, подбираем более-менее подходящие структуры: список, двусвязный список ну и массив, и тестируем их на нужных операциях. В данном случае требуется всего то вставка/извлечение в голову/хвост, сортировка нас тут не интересует, а если очередь имеет переменную длину от короткой до очень большой, да еще и элементы крупные, то вектор тут нас вряд ли устроит в силу перевыделения памяти, со всеми вытекающими. Пример 2. А вот скажем если стоит задача в организации некой структуры, которая будет работать в качестве страничного кэша с учетом частоты использования, то тут мы скорее склонимся к выбору вектора, т.к. кэш скорее всего будет иметь фиксированный размер, а так же нам потребуется часто вызывать сортировку (перемещение) блоков памяти (элементов вектора) по релевантности и т.д. (Прошу не пинать, ясно что это не единственный вариант реализации) Отсюда определяем операции со структурой и тестируем на них кандидатов. Синтетика синтетикой, но без фанатизма. Я считаю, что надо задавать внешние условия применимости, а не накладывать внутренние ограничения реализации, потому что тогда выходит какая то чушь, это как взять примус и колотить им гвозди.

zasyadko
() автор топика

Пересел с Си на Плюсы. Написал для своих задач реализацию листа

Уходи обратно на Си :)

slackwarrior ★★★★★
()
Ответ на: комментарий от buddhist

Генератор карт для Heroes of Might and Magic 2 уже тоже написали?

dzidzitop ★★
()
Ответ на: комментарий от anonymous

Гонишь. Везде где помню дефолтный new реализован поверх malloc. Соответственно, сишный связный список будет вести себя так же.

dzidzitop ★★
()
Ответ на: комментарий от pftBest

Вектор увеличивается вдвое (вроде бы стандарт обязывает, но не помню точно обязывает ли). А узел std::list занимает +2 указателя (prev/next).

dzidzitop ★★
()
Ответ на: комментарий от invy

Всё это зависит от такого числа параметров, что в отрыве от конкретной системы такие тесты вообще смысла не имеют. При этом связные списки почти никогда нормальный человек использовать не будет за отсутствием необходимости. Но очень редко надо, да. Или иногда не хочется писать громоздкий код на массиве.

dzidzitop ★★
()
Ответ на: комментарий от zasyadko

только переопределил некоторые методы, например получение размера и проверку на пустоту

Вроде C++11 уже требует O(1) на эти операции. Ну или какой-то то там более свежий.

dzidzitop ★★
()
Ответ на: комментарий от ncuxer

И что ему будет если он декоратор написал? Или какой-то хрен с горы сказал - и надо следовать заповедям не включая мозг?

dzidzitop ★★
()
Ответ на: комментарий от anonymous

да. никаких ограничений вообще нет.

но можно надеяться на адекватную имплементацию

lists operations that are provided for some types of sequence containers but not others. An
implementation shall provide these operations for all container types shown in the “container” column, and shall implement them so as to take amortized constant time

и не совсем понятно какой смысл накладывает стандарт на слово shall. если это типа распоряжения, то допускается любой (переменный) коэффициент роста capacity, лишь бы он был больше 1.

dzidzitop ★★
()
Последнее исправление: dzidzitop (всего исправлений: 3)
Ответ на: комментарий от dzidzitop

А также допускается конечное число применений роста capacity на конечную величину, если это конечное число в первом приближении много меньше эффективного размера вектора.

Короче сишным и сипласпласным стандартами нужно авторов бить по голове. Дабы неповадно было писать чушь, из-за которой портабельный код писать невозможно.

dzidzitop ★★
()
Последнее исправление: dzidzitop (всего исправлений: 1)
Ответ на: комментарий от dzidzitop

и не совсем понятно какой смысл накладывает стандарт на слово shall

Как долженствование.

anonymous
()
Ответ на: комментарий от PPP328

абстракциям юзать системные вызовы malloc/free

Вам лучше не подходить к си и плюсам. Нет таких сисколов на большинстве систем. Есть sbrk и mmap или аналоги. malloc/free реализованы в конкретной libc.

grossws
()
Ответ на: комментарий от grossws

Под системными вызовами я подразумеваю вызов из системных библиотек в отличие от пользовательских библиотек.

PPP328 ★★★★★
()
Ответ на: комментарий от PPP328

из системных библиотек в отличие от пользовательских библиотек

Странное разделение, трудно провести границу.

i-rinat ★★★★★
()
12 июля 2017 г.
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.