powered by simpleCommunicator - 2.0.61     © 2026 Programmizd 02
Целевая тема:
Создать новую тему:
Автор:
Закрыть
Цитировать
Форумы / Java [игнор отключен] [закрыт для гостей] / Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
25 сообщений из 140, страница 2 из 6
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515019
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
Код: java
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
14.
15.
16.
17.
18.
19.
20.
21.
22.
23.
24.
25.
26.
27.
28.
29.
30.
31.
32.
33.
34.
35.
36.
37.
38.
39.
40.
41.
42.
43.
44.
45.
public class ThreadPoolImpl implements ThreadPool {
    private Logger logger = LoggerFactory.getLogger(ThreadPoolImpl.class);
    private final ConcurrentMap<String, CompletableFuture<Void>> taskMap = new ConcurrentHashMap<>();
    private volatile boolean isStopped = false;
    private final ThreadPoolExecutor poolExecutor;

    public ThreadPoolImpl(int threadCount) {
        poolExecutor = new ThreadPoolExecutor(threadCount, threadCount, 60, TimeUnit.SECONDS,
                new LinkedBlockingQueue<>());
    }

    @Override
    public void execute(MyTask myTask) {
        if (isStopped) {
            throw new IllegalStateException("Pool already terminated");
        }
        final String key = myTask.getKey();
        taskMap.compute(key, (k, queue) -> {
            logger.info("Key " + key + " compute started");
            CountDownLatch countDownLatch = new CountDownLatch(1);
            CompletableFuture<Void> future = (queue == null)
                    ? CompletableFuture.runAsync(() -> {
                myTask.run();
                try {
                    countDownLatch.await();
                } catch (InterruptedException e) {
                    logger.error("Thread pool thread was interrupted", e);
                }
            }, poolExecutor)
                    : queue.whenCompleteAsync((r, e) -> {
                myTask.run();
                try {
                    countDownLatch.await();
                } catch (InterruptedException ex) {
                    logger.error("Thread pool thread was interrupted", ex);
                }
            }, poolExecutor);
            //to prevent OutOfMemoryError in case if we will have too much keys
            future.whenComplete((r, e) -> taskMap.remove(key, future));
            countDownLatch.countDown();        
            logger.info("Key " + key + " compute finished");

            return future;
        });
    }



Вот кстати рабочий вариант, который исключает ситуацию, что задача закончилась до того, как мы прилепили ей коллбэк
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515023
Alexey Tomin
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
questioner
Код: java
1.
2.
3.
4.
        taskMap.compute(key, (k, queue) -> {
...
                    countDownLatch.await();
...


Вот кстати рабочий вариант, который исключает ситуацию, что задача закончилась до того, как мы прилепили ей коллбэк

Т.е. опять- пишут "метод должен отрабатывать как можно быстрее", а тут await.
Не, я понимаю, тут может всё достаточно быстро, но нафига писать такой код? Его трудно понимать, трудно поддерживать.
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515025
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
Alexey TominquestionerДа и заказчику зачастую на%?#ть понятно написано или нет - лишь бы работало.

Вас оценивали потенциальные коллеги. Они не заказчики, им важна понятность.


По разному бывает.

Вообще странно как-то писать, что у вас не работает корректно детально то-то и то-то, а по факту подразумевать, что код не понятен. Не находите?
questionerпропущено...
Alexey TominПочему?

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

То, что Вы не смогли это сразу понять ещё не значит, что это непонятно написано.

Скажем есть список рукописный и у вас есть ссылка на голову. Нельзя назвать эту ссылку list ? Этот аргумент по сути является очередью задач, а для меня ожидателем выполнения всей очереди до этого элемента включительно. Именование дело такое... как не назови - все не поймут, но мне кажется я постарался.
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515027
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
Alexey Tominquestioner
Код: java
1.
2.
3.
4.
        taskMap.compute(key, (k, queue) -> {
...
                    countDownLatch.await();
...


Вот кстати рабочий вариант, который исключает ситуацию, что задача закончилась до того, как мы прилепили ей коллбэк

Т.е. опять- пишут "метод должен отрабатывать как можно быстрее", а тут await.
Не, я понимаю, тут может всё достаточно быстро, но нафига писать такой код? Его трудно понимать, трудно поддерживать.

Предложите вариант получше? я не смог просто. Думаете я стараюсь посложнее специально написать?
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515053
Alexey Tomin
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
questionerAlexey Tominпропущено...
Вас оценивали потенциальные коллеги. Они не заказчики, им важна понятность.

По разному бывает.

Не понял. Что, кто-то любит непонятный код в репозитории? Не знаю, я бы не хотел там работать...

questionerВообще странно как-то писать, что у вас не работает корректно детально то-то и то-то, а по факту подразумевать, что код не понятен. Не находите?

1. Я оцениваю код, а не тех, кто оценивал код
2. Может там и ошибка. Мне не хочется разбираться.

questionerСкажем есть список рукописный и у вас есть ссылка на голову. Нельзя назвать эту ссылку list ?

head, по-моему, правильнее. Например потом, что queue не может быть null. А head - может.

questionerПредложите вариант получше? я не смог просто. Думаете я стараюсь посложнее специально написать?

Не знаю. В соседней теме я показал то, что написал за неполный час.
По мне- так намного проще. Возможно чуть медленнее, но для начала точно хватит.
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515071
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
Alexey Tominquestionerпропущено...

По разному бывает.

Не понял. Что, кто-то любит непонятный код в репозитории? Не знаю, я бы не хотел там работать...


Этот комментарий относился к тому, что иногда оценивают коллеги, а иногда и заказчику могут заслать.

Alexey TominquestionerВообще странно как-то писать, что у вас не работает корректно детально то-то и то-то, а по факту подразумевать, что код не понятен. Не находите?

1. Я оцениваю код, а не тех, кто оценивал код
2. Может там и ошибка. Мне не хочется разбираться.

Собственно суть топика в том, чтобы обсудить аргументацию. Да, дополнительно обсудить какие-то другие решения тоже интересно.


Alexey TominquestionerСкажем есть список рукописный и у вас есть ссылка на голову. Нельзя назвать эту ссылку list ?

head, по-моему, правильнее. Например потом, что queue не может быть null. А head - может.

Согласен, но это название допустимо всё же.
Alexey TominquestionerПредложите вариант получше? я не смог просто. Думаете я стараюсь посложнее специально написать?

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

Для какого начала? это тестовое задание) Тем более вариант попроще и попонятнее я и так имею и он был вместе с этим отправлен. То, что там могут быть коллизии ребятам не понравилось.


EmptyLock не используется у вас нигде кроме комментариев. зачем он?
что такое "conflict" error ?

Ваша имплементация нарушает требования тестового задания, что все задачи должны выполняться в порядке прихода. Это на первый взгляд
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515079
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
Alexey Tomin,

Код: java
1.
executorService.submit(() -> executeTask(task));


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

То есть подряд идущие
key1
key1
key2

займут 3 потока вместо 2-ух. Но я думаю Вы это понимаете. Первое моё решение как-раз ругали из-за подобной "неоптимальности"
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515097
Alexey Tomin
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
questionerЭтот комментарий относился к тому, что иногда оценивают коллеги, а иногда и заказчику могут заслать.

Заказчик либо не читает, либо тоже смотрит понятность кода.

questionerAlexey TominНе знаю. В соседней теме я показал то, что написал за неполный час.
По мне- так намного проще. Возможно чуть медленнее, но для начала точно хватит.

EmptyLock не используется у вас нигде кроме комментариев. зачем он?
что такое "conflict" error ?

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

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

В любом случае- не надо использовать методы Map для того, чтобы ставить задания в очередь. Это не то.

Их аргументацию обсуждать мне не интересно, т.к. они здесь не пишут.
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515116
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
Alexey TominquestionerЭтот комментарий относился к тому, что иногда оценивают коллеги, а иногда и заказчику могут заслать.

Заказчик либо не читает, либо тоже смотрит понятность кода.

questionerпропущено...


EmptyLock не используется у вас нигде кроме комментариев. зачем он?
что такое "conflict" error ?

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

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

В любом случае- не надо использовать методы Map для того, чтобы ставить задания в очередь. Это не то.

Их аргументацию обсуждать мне не интересно, т.к. они здесь не пишут.

Им были заданы вопросы, на которые им некогда отвечать) Так что считаю обсудить тут вполне корректно. К тому же имён я не раскрывал.

А зачем вы скинули пример теста если мы сейчас обсуждаем имплементацию экзекутора?

Ну в общем суть в том, что "для начала сойдёт" - не сошло) Как я уже писал - решение с подобным перформансом было и оно не удовлетворило проверяющих.
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515147
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
questionerТолько они этого не нашли)

Надо подумать как это решить... из спортивного интереса

Код: java
1.
whenCompleteAsync() ?
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515151
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
questioner,

Что-то я не понимаю как должен taskMap.remove() вообще работать в приведенном коде? Он же на каждую задачу в цепочке будет вызываться, даже если там есть ещё задачи, мы всю цепочку по ключу удалим?
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515159
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
BlazkowiczquestionerТолько они этого не нашли)

Надо подумать как это решить... из спортивного интереса

Код: java
1.
whenCompleteAsync() ?


Да, годится. По коду так попроще конечно будет и никаких новых race condition от этого я не вижу.
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515161
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
Alexey TominНа самом деле имя "queue" для этого параметра- уже смертный приговор.
Поддержу. В одном методе два разных Future и у них абсолютно ни о чем не говорящие имена queue и future. Почему было бы не назвать last/tail и next, например?
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515162
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
Blazkowiczquestioner,

Что-то я не понимаю как должен taskMap.remove() вообще работать в приведенном коде? Он же на каждую задачу в цепочке будет вызываться, даже если там есть ещё задачи, мы всю цепочку по ключу удалим?

task.remove удаляет не по ключу, а по ключу и значению.

Если значение(фьюча) поменялась, то ничего удалится
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515164
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
questioner,


questionerЕсли значение(фьюча) поменялась, то ничего удалится
Если значение(фьюча) поменялась, то ничего НЕ удалится
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515166
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
Blazkowicz,

Это на мой взгляд самая хитрая часть этого кода и было потрачено много времени, чтобы придумать эту конструкцию
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515167
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
questionerЕсли значение(фьюча) поменялась, то ничего НЕ удалится
Точно. Спасибо.
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515183
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
questionerВ jdk ConcurrentHashMap compute не может исполняться Параллельно для ключей из одного бакета.
Этим мы расписываемся в том что решение не устойчиво к атаке на hash collision?
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515184
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
Alexey TominВас оценивали потенциальные коллеги. Они не заказчики, им важна понятность.

Да, стоило написать явно на чем именно ключи синхронизируются.
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515206
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
BlazkowiczquestionerВ jdk ConcurrentHashMap compute не может исполняться Параллельно для ключей из одного бакета.
Этим мы расписываемся в том что решение не устойчиво к атаке на hash collision?

Это же инфраструктурный класс. Кому придёт в голову его атаковать?

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

То, что всё это работает на очереди фьючей? комментарии в коде имеете ввиду или что-то типа сопроводительного письма?
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515214
забыл ник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
questioner
Это же инфраструктурный класс. Кому придёт в голову его атаковать?

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


Чисто теоретически можно деградировать вашу систему до sequential состояния, а это уже косяк. Не агритесь, вам дело говорят. Да и вообще интервью с заданием на многопоточность это лотерея, ну не прошли и ничего страшного, я тоже из 3 одно прохожу хоть и опыта хватает)
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515216
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
questionerЭто же инфраструктурный класс. Кому придёт в голову его атаковать?

Мы не знаем источника ключей.

questionerК тому же ничего уж сильно плохого не случится. Добавление в мапу и создание парочки фьючей не такая уж и тяжёлая операция по сравнению с самой задачей.

Ну, тогда бы и решение с synchronized подошло бы?

questionerТо, что всё это работает на очереди фьючей? комментарии в коде имеете ввиду или что-то типа сопроводительного письма?
В документации сказано что compute атомарный и всё. Стоило указать в коде что мы полагаемся на это поведение, ожидая синхронизацию для ключа исходя из атомарности.
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515224
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
questioner,

Придется ждать vimba, я тоже не вижу как параллельно запустить задачи с одним ключом.
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515228
questioner
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Гость
BlazkowiczquestionerЭто же инфраструктурный класс. Кому придёт в голову его атаковать?

Мы не знаем источника ключей.

questionerК тому же ничего уж сильно плохого не случится. Добавление в мапу и создание парочки фьючей не такая уж и тяжёлая операция по сравнению с самой задачей.

Ну, тогда бы и решение с synchronized подошло бы?


Вы утрируете.

Synchronized - всё в один поток и многопоточности нет тогда.
допустим у нас большой объём разных ключей, которые валятся в один бакет, но при этом они в основном разные.
Грубо говоря можно представить, что в данной есть один тред, который занимается маршрутизацией, то есть создаёт фьючи только, а сами таски выполняются в экзекуторе в отдельных тредах. Или Вы про Collections.synchronizedMap ? тогда да, то же самое. Но это на мой взгляд не такая уж и большая проблема.

Не очень понял Ваш пассаж. Что Вы имеете ввиду?

BlazkowiczquestionerТо, что всё это работает на очереди фьючей? комментарии в коде имеете ввиду или что-то типа сопроводительного письма?
В документации сказано что compute атомарный и всё. Стоило указать в коде что мы полагаемся на это поведение, ожидая синхронизацию для ключа исходя из атомарности.

автор Some attempted update operations on this map by other threads
may be blocked while computation is in progress, so the
computation should be short and simple, and must not attempt to
update any other mappings of this Map.

Согласен, что явно этого не сказано, но намёк очень явен. В принципе да, согласен, если бы написал - они бы уточнили, проверили и не написали бы то, что написали
...
Рейтинг: 0 / 0
Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
    #39515231
Фотография Blazkowicz
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
questionerSynchronized - всё в один поток и многопоточности нет тогда.

Ну, при коллизии hash кодов у нас будет compute всегда synchronized. То есть распределение таски по очередям полностью синхронизировано. Разницы вот с этим кодом нет:

Код: java
1.
2.
3.
synchronized(this) {
  taskMap.compute(lambda);
}
...
Рейтинг: 0 / 0
25 сообщений из 140, страница 2 из 6
Форумы / Java [игнор отключен] [закрыт для гостей] / Сказали, что выполнил тестовое задание неправильно, я с этим не согласен. Кто прав?
Найденые пользователи ...
Разблокировать пользователей ...
Читали форум (0):
Пользователи онлайн (0):
x
x
Закрыть


Просмотр
0 / 0
Close
Debug Console [Select Text]