-
Notifications
You must be signed in to change notification settings - Fork 0
Hw1 thread pool #2
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
base: master
Are you sure you want to change the base?
Conversation
Simple test for threadPool was added
| @Override | ||
| public <U> LightFuture<U> thenApply(@NotNull final Function<T, ? extends U> f) { | ||
| final Supplier<U> supplier = () -> f.apply(Task.this.get()); | ||
| return addTask(supplier); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
" Метод thenApply не должен блокировать работу потока, если результат задачи X ещё не вычислен", -1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нужно добавить тест, который это проверяет
| if (!isWorking) { | ||
| throw new ThreadPoolIsTurnedDownException(); | ||
| } | ||
| if (supplier == POISON_PILL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вся эта машинерия с POISON_PILL -- это по сути хак, от которого код ни в одном месте не становится понятней. -0.5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Предлагаю просто сделать булевый флаг
|
Добрый день.
Ровно две Ваши претензии я уточнял у Юрия. Переписку прикладываю.
пн, 30 апр. 2018 г., 15:08 Denis Zharkov <notifications@github.com>:
… ***@***.**** requested changes on this pull request.
------------------------------
In
ThreadPool/src/main/java/com/spbau/mit/kirakosian/threadPool/ThreadPoolImpl.java
<#2 (comment)>:
> + }
+ if (error != null) {
+ throw new LightExecutionException(error);
+ }
+ return value;
+ }
+
+
+ /**
+ * ***@***.*** LightFuture#thenApply(Function)}
+ */
+ @nullable
+ @OverRide
+ public <U> LightFuture<U> ***@***.*** final Function<T, ? extends U> f) {
+ final Supplier<U> supplier = () -> f.apply(Task.this.get());
+ return addTask(supplier);
" Метод thenApply не должен блокировать работу потока, если результат
задачи X ещё не вычислен", -1
------------------------------
In
ThreadPool/src/main/java/com/spbau/mit/kirakosian/threadPool/ThreadPoolImpl.java
<#2 (comment)>:
> + // do nothing
+ }
+ }
+ }
+ }
+
+ /**
+ * ***@***.*** ThreadPool#addTask(Supplier)}
+ */
+ @nullable
+ @OverRide
+ public synchronized <T> LightFuture<T> addTask(final Supplier<T> supplier) {
+ if (!isWorking) {
+ throw new ThreadPoolIsTurnedDownException();
+ }
+ if (supplier == POISON_PILL) {
Вся эта машинерия с POISON_PILL -- это по сути хак, от которого код ни в
одном месте не становится понятней. -0.5
------------------------------
In
ThreadPool/src/main/java/com/spbau/mit/kirakosian/threadPool/ThreadPoolImpl.java
<#2 (comment)>:
> + // do nothing
+ }
+ }
+ }
+ }
+
+ /**
+ * ***@***.*** ThreadPool#addTask(Supplier)}
+ */
+ @nullable
+ @OverRide
+ public synchronized <T> LightFuture<T> addTask(final Supplier<T> supplier) {
+ if (!isWorking) {
+ throw new ThreadPoolIsTurnedDownException();
+ }
+ if (supplier == POISON_PILL) {
Предлагаю просто сделать булевый флаг
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AP3e70HQCYhxk0FuUUX8NSzJMMV-uLqCks5ttwzJgaJpZM4SgdHR>
.
|
|
<<Вопрос про ThreadPool.
Есть рекомендация про способ остановки потоков.
Можно ли дождаться завершения задач, а после остановить потоки
(POISION_PILL pattern)?>>
<<Здравствуйте.
Да, такой вариант вполне хорош. Но надо какой-то разумный таймаут на
случай, если задача зависла.>>
<<Не очень понятно, как можно прервать работу потока при помощи interrupt.
К примеру, он находился в состоянии wait ожидая чего-либо, а после выхода
сделал стандартную проверку на ожидаемое условие и опять уснул (либо
перешел к следующей задаче).
Вероятнее всего, такое прерывание приведет к падению с ошибкой (если задачи
были зависимы и.т.д), так что мне кажется разумным оставить на совести
пользователя зависшую задачу, т.к. в общем случае вроде как нет возможности
остановить ее выполнение.
Возможно, я не совсем правильно понимаю принцип работы interrupt, но судя
по моим экспериментам описанная мной ситуация вполне реальна.>>
<<Совсем убить задачу можно через Thread.stop(), только пользоваться им не
рекомендуется, потому что это может привести всю систему в неконсистентное
состояние, так что да, давайте зависшую задачу оставим на совести
пользователя. interrupt можно использовать, чтобы заставить поток проверить
флажок, который скажет ему, что пора завершаться --- естественно, при
условии, что внутри задачи никто ничего с interrupt не делает. Тогда, когда
мы вызываем shutdown, пул потоков может выставлять этот флажок, очищать
очередь задач (и заставить их забыть свои thenApply) и делать interrupt
всем потокам.>>
<<Есть еще один вопрос. Про thenApply.
В задании сказано, что "Метод thenApply не должен блокировать работу
потока, если результат задачи X ещё не вычислен". Про работу какого потока
говорится? Того, который вызвал thenApply, или того, который наченет
выполнять задачу?>>
<<Ага, того, который вызвал thenApply. Но создавать и блокировать поток, в
котором будет выполняться задача thenApply, до получения результата
основной задачи --- не очень хорошо. Может оказаться существенно больше
потоков, чем нужно. Надо, чтобы thenApply порождала задачу, которая просто
ставится в очередь пула и работает как обычные задачи.>>
Дополнительно, poison_pill является стандартнцм простым способом остановки
потоков.
пн, 30 апр. 2018 г., 16:16 Alexander Kirakosyan <kair1998@gmail.com>:
… Добрый день.
Ровно две Ваши претензии я уточнял у Юрия. Переписку прикладываю.
пн, 30 апр. 2018 г., 15:08 Denis Zharkov ***@***.***>:
> ***@***.**** requested changes on this pull request.
> ------------------------------
>
> In
> ThreadPool/src/main/java/com/spbau/mit/kirakosian/threadPool/ThreadPoolImpl.java
> <#2 (comment)>
> :
>
> > + }
> + if (error != null) {
> + throw new LightExecutionException(error);
> + }
> + return value;
> + }
> +
> +
> + /**
> + * ***@***.*** LightFuture#thenApply(Function)}
> + */
> + @nullable
> + @OverRide
> + public <U> LightFuture<U> ***@***.*** final Function<T, ? extends U> f) {
> + final Supplier<U> supplier = () -> f.apply(Task.this.get());
> + return addTask(supplier);
>
> " Метод thenApply не должен блокировать работу потока, если результат
> задачи X ещё не вычислен", -1
> ------------------------------
>
> In
> ThreadPool/src/main/java/com/spbau/mit/kirakosian/threadPool/ThreadPoolImpl.java
> <#2 (comment)>
> :
>
> > + // do nothing
> + }
> + }
> + }
> + }
> +
> + /**
> + * ***@***.*** ThreadPool#addTask(Supplier)}
> + */
> + @nullable
> + @OverRide
> + public synchronized <T> LightFuture<T> addTask(final Supplier<T> supplier) {
> + if (!isWorking) {
> + throw new ThreadPoolIsTurnedDownException();
> + }
> + if (supplier == POISON_PILL) {
>
> Вся эта машинерия с POISON_PILL -- это по сути хак, от которого код ни в
> одном месте не становится понятней. -0.5
> ------------------------------
>
> In
> ThreadPool/src/main/java/com/spbau/mit/kirakosian/threadPool/ThreadPoolImpl.java
> <#2 (comment)>
> :
>
> > + // do nothing
> + }
> + }
> + }
> + }
> +
> + /**
> + * ***@***.*** ThreadPool#addTask(Supplier)}
> + */
> + @nullable
> + @OverRide
> + public synchronized <T> LightFuture<T> addTask(final Supplier<T> supplier) {
> + if (!isWorking) {
> + throw new ThreadPoolIsTurnedDownException();
> + }
> + if (supplier == POISON_PILL) {
>
> Предлагаю просто сделать булевый флаг
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#2 (review)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AP3e70HQCYhxk0FuUUX8NSzJMMV-uLqCks5ttwzJgaJpZM4SgdHR>
> .
>
|
|
|
С этим я согласен, и потратил довольно много времени на попытки создания
неблокирующей реализации. К сожалению, я не нашел способа, который может
разумно сработать.
Основная проблема в том, что один раз начав выполнять задачу мы не можем
еще раз положить ее в пул, т.к. наш указатель на фючер потеряется (мы будем
думать, что уже выполнили задачу, хотя на самом деле нет).
пн, 30 апр. 2018 г., 15:29 Denis Zharkov <notifications@github.com>:
…
1. Про poison pill на ваше усмотрение тогда
2. "Надо, чтобы thenApply порождала задачу, которая просто
ставится в очередь пула и работает как обычные задачи." кажется, что
неправильно блокировать потоки заведомо заблокированной задачей
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AP3e79QW7_1ASFc5vVdAsO9WMbfZl66hks5ttxGkgaJpZM4SgdHR>
.ш
|
|
Там же простая идея: храним в родительской таске список дочерних и кладем их в общую очередь после окончания родительской |
|
В таком случае задача, которая зависит от уже выполненной не будет никогда
добавлена в пул.
пн, 30 апр. 2018 г., 15:48 Denis Zharkov <notifications@github.com>:
… Там же простая идея: храним в родительской таске список дочерних и кладем
их в общую очередь после окончания родительской
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AP3e76qDcdw9ZK6_PRj4gdR1-W94Z8zpks5ttxYagaJpZM4SgdHR>
.
|
|
Хотя эта проблеиа решается... Ладно, тогда согласен, хотя хочется еще про
это подумать.
пн, 30 апр. 2018 г., 16:49 Alexander Kirakosyan <kair1998@gmail.com>:
… В таком случае задача, которая зависит от уже выполненной не будет никогда
добавлена в пул.
пн, 30 апр. 2018 г., 15:48 Denis Zharkov ***@***.***>:
> Там же простая идея: храним в родительской таске список дочерних и кладем
> их в общую очередь после окончания родительской
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#2 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AP3e76qDcdw9ZK6_PRj4gdR1-W94Z8zpks5ttxYagaJpZM4SgdHR>
> .
>
|
No description provided.