Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Ready] Fast Concurrent Deque Through Explicit Timestamping #127

Open
wants to merge 30 commits into
base: integration
Choose a base branch
from

Conversation

EduardBlees
Copy link

Implementation of [2014] Dodds,Haas,Kirsch Fast Concurrent Data-Structures Through Explicit Timestamping
Выполняли: Чирухин, Блеес (3304), Саклакова (3303)

@EduardBlees EduardBlees changed the title Fast Concurrent Deque Through Explicit Timestamping [Ready] Fast Concurrent Deque Through Explicit Timestamping Dec 28, 2018
@EduardBlees
Copy link
Author

@eugenyk

@pr3sto
Copy link

pr3sto commented Dec 28, 2018

cтресс тесты не собирались из-за того, что несколько раз пересобирал разными версиями gcc. пересобрал с нуля одной версией - все заработало

@eugenyk
Copy link
Contributor

eugenyk commented Jan 4, 2019

@EduardBlees - пока тесты не проходят для большей части архитектур

@pr3sto
Copy link

pr3sto commented Jan 4, 2019

@eugenyk #6 не собирается ассемблерный код под архитектуру i386, это поправим.

#7 и #4 выдают ошибку Exception: Illegal. Причем ошибка не только в стресс, но и в юнит тестах. По ошибке абсолютно не понятно, чем она вызвана. Я могу только предположить: у нас ассемблерный код написан только для двух архитектур: ts_hardwaretimestamp.h. И тесты используют платформозависимый HardwareTimestamp класс. Можно тесты переписать на AtomicCounterTimestamp, который не зависит от платформы и компилятора.

@khizmax
Copy link
Owner

khizmax commented Jan 4, 2019

@eugenyk #6 не собирается ассемблерный код под архитектуру i386, это поправим.

FYI - здесь собран зоопарк реализаций rdtsc для разных архитектур:
https://github.com/google/benchmark/blob/v1.1.0/src/cycleclock.h#L56

@eugenyk
Copy link
Contributor

eugenyk commented Jan 5, 2019

Можно тесты переписать на AtomicCounterTimestamp, который не зависит от платформы и компилятора

@pr3sto по-хорошему надо бы в зависимости от платформы определять тип счётчика и для нереализованных действително использовать кроссплатформенный, через traits его передать в контейнер. Тут Максим меня может поправить

@khizmax
Copy link
Owner

khizmax commented Jan 5, 2019

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

@eugenyk
Copy link
Contributor

eugenyk commented Jan 8, 2019

выдают ошибку Exception: Illegal

@pr3sto, это ошибка вызова некорректной инструкции, ниже прилагаю содержимое core dump, полученного после запуска модульного теста - в общем ваше предположение верно в части счётчика

Core was generated by `./unit-deque'.
Program terminated with signal SIGILL, Illegal instruction.
#0  0x000000000046eb16 in void (anonymous namespace)::TSDeque::test<cds::container::TSDeque<int, cds::container::HardwareTimestamp, cds::opt::item_counter<cds::atomicity::item_counter>::pack<cds::container::tsdeque::traits> > >(cds::container::TSDeque<int, cds::container::HardwareTimestamp, cds::opt::item_counter<cds::atomicity::item_counter>::pack<cds::container::tsdeque::traits> >&) [clone .isra.183] [clone .constprop.228] ()
(gdb) bt
#0  0x000000000046eb16 in void (anonymous namespace)::TSDeque::test<cds::container::TSDeque<int, cds::container::HardwareTimestamp, cds::opt::item_counter<cds::atomicity::item_counter>::pack<cds::container::tsdeque::traits> > >(cds::container::TSDeque<int, cds::container::HardwareTimestamp, cds::opt::item_counter<cds::atomicity::item_counter>::pack<cds::container::tsdeque::traits> >&) [clone .isra.183] [clone .constprop.228] ()
#1  0x000000000046fe09 in (anonymous namespace)::TSDeque_hardware_timestamping_Test::TestBody() ()
#2  0x000000000049a263 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ()
#3  0x0000000000491a64 in testing::Test::Run() [clone .part.368] ()
#4  0x0000000000491d1d in testing::TestInfo::Run() [clone .part.369] ()
#5  0x0000000000491f05 in testing::TestCase::Run() [clone .part.370] ()
#6  0x00000000004943b1 in testing::internal::UnitTestImpl::RunAllTests() [clone .part.371] [clone .constprop.426] ()
#7  0x00000000004947c1 in testing::UnitTest::Run() ()
#8  0x000000000043e7e6 in main ()

@pr3sto
Copy link

pr3sto commented Jan 16, 2019

@eugenyk по этой ссылке есть только инструкция rdtsc. у нас используются: rdtsc и rdtscp. поэтому для других архитектур и компиляторов не добавил реализацию (для i386 поправил).
тесты платформозависимых версий теперь только для архитектур, для которых есть реализация:

#if defined(CDS_ts_hardwaretimestamp_hwptime_defined) && defined(CDS_ts_hardwaretimestamp_hwtime_defined)

@eugenyk
Copy link
Contributor

eugenyk commented Jan 17, 2019

@pr3sto, тест всё равно не проходит под i386

@pr3sto
Copy link

pr3sto commented Jan 17, 2019

@eugenyk я не вижу другого выхода для нас, кроме как удалить поддержку i386. У нас нет возможности тестировать и отлаживать код для этой платформы, а работа вслепую, как видим, результатов не приносит. Тем более, что у нас написан платформонезависимый atomiccountertimestamp, который работает абсолютно также, да и от алгоритма взятия временных меток работа дека никак не зависит

@eugenyk
Copy link
Contributor

eugenyk commented Jan 17, 2019

@eugenyk я не вижу другого выхода для нас, кроме как удалить поддержку i386. У нас нет возможности тестировать и отлаживать код для этой платформы, а работа вслепую, как видим, результатов не приносит. Тем более, что у нас написан платформонезависимый atomiccountertimestamp, который работает абсолютно также, да и от алгоритма взятия временных меток работа дека никак не зависит

Если вы попросите доступ - у вас будет возможность отлаживать код и под этой архитектурой, это не проблема. В любом случае, поддерживаться должны все архитектуры, которые сейчас поддерживаются библиотекой - пусть и через платформонезависмый счётчик будет поддерживаться i386, должно собираться и работать

@pr3sto
Copy link

pr3sto commented Jan 18, 2019

@eugenyk последняя попытка пофиксить HardwareTimestamp. Скорее всего дело не в архитектуре, а в том, что у процессора просто нет инструкции rdtscp (по заявлениям авторов статьи, она есть на всех современных x86 процессорах). Добавил проверку (как описано тут)

@@ -22,6 +23,15 @@ namespace cds { namespace tshardwaretimestamp {
return ret;
}

static inline int has_rdtscp()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's quit strange to have "has_..." function returning logically boolean value with int signature


deque_type dq(1, 0);
test( dq );
if (cds::tshardwaretimestamp::platform::has_rdtscp() == 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You check rdtscp existence only in unit tests, but what should do developers who will use your API, the same? It should be transparent for users

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Проблема в том, что, как я понял, нет возможности проверить наличие rdtscp во время компиляции. Эту проверку необходимо делать во время выполнения программы. Я не знаю, как сделать это прозрачным для пользователей. Подскажите?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's possible to determine it through cpuid like this

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не совсем понимаю, что вы имеете ввиду. У нас и так проверка наличия rdtscp происходит с помощью cpuid. И функция cds::tshardwaretimestamp::platform::has_rdtscp() возвращает 1, если процессор поддерживает rdtscp.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Всё верно, я сначала ответил на вопрос, а потом посмотрел реализацию

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants