Skip to content

Добавила решение 3 домашней работы#3

Open
Nigma-Ks wants to merge 7 commits intomainfrom
hw3
Open

Добавила решение 3 домашней работы#3
Nigma-Ks wants to merge 7 commits intomainfrom
hw3

Conversation

@Nigma-Ks
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown

@yurii-litvinov yurii-litvinov left a comment

Choose a reason for hiding this comment

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

Полностью повторяет решение https://github.com/AnNyiiik/HWThirdTerm/pull/3/files#diff-4bad84ce6514350bfacc97c66857ef336944fc356d7a01ebc4f3edb3b6478150. Все списанные домашние работы надо будет полностью с нуля переделать и показывать вживую, умея отвечать на вопросы по решению и вносить правки.

Copy link
Copy Markdown

@yurii-litvinov yurii-litvinov left a comment

Choose a reason for hiding this comment

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

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

Comment on lines +6 to +7
readonly int amountOfThreads = 5;
MyThreadPool threadPool;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Надо модификаторы видимости

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не поправлено

@@ -0,0 +1,95 @@

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

var myTask = threadPool.Submit(() =>
{
throw new InvalidOperationException();
return 4; //without this string test doesn't work
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тут, скорее всего, проблема с тем, что без return тип лямбды не вывести. Можно добавить явную аннотацию типа: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/lambda-expressions#explicit-return-type


Assert.That(values.Count, Is.EqualTo(amountOfThreads));
}
} No newline at end of file

This comment was marked as resolved.

@@ -0,0 +1,24 @@
using static Homework3.MyThreadPool;

public interface IMyTask<T>

This comment was marked as resolved.

Comment on lines +132 to +135
public bool IsCompleted()
{
return isResultReady;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
public bool IsCompleted()
{
return isResultReady;
}
public bool IsCompleted()
=> isResultReady;

{
if (!isResultReady)
{
myTaskThreadHandler.Reset();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WaitOne?

}
if (Volatile.Read(ref exception) == null)
{
return result;

This comment was marked as resolved.

public MyTask(MyThreadPool myThreadPool, Func<T> function)
{
this.myThreadPool = myThreadPool;
this.myThreadPool.taskQueque.Enqueue(new Action(() =>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Вообще, лучше, чтобы сам объект делал свою работу. Тут на самом деле тредпулу надо много всякой внутренней синхронизации проделать (у Вас пока потребность в этом не возникла, потому что пока что он делает совсем не то, что должен), и MyTask это очень тяжело будет сделать правильно.

{
if (!myThreadPool.cancellationTokenSource.IsCancellationRequested)
{
var nextFunction = new Func<T2>(() => continueFunction(Result()));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Опять-таки, никто не обещал, что в этом месте Shutdown ещё не отработал

{
tasks[localI] = threadPool.Submit(() =>
{
manualResetEvent.WaitOne();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Нам было бы интереснее попробовать Submit делать одновременно, а не задачи, уже поставленные на пул, одновременно стартовать

{
Assert.That(tasks[i].Result(), Is.EqualTo(4));
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Много потоков, делающих Submit, проверяется, но единственный ли это источник потенциальных гонок в этой задаче?

Comment on lines +20 to +22
/// <typeparam name="T2"></typeparam>
/// <param name="continueFunction"></param>
/// <returns></returns>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не поправлено

{
thread.Start();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Перед } пустая строка не ставится

public class MyThreadPool
{
private readonly int timeForEndingWork = 1000;
private readonly object cancellationTokenLockObject = new();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Странное название, будто этот объект имеет какое-то отношение к cancellationToken именно

Comment on lines +68 to +69


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change

{
taskQueue.Enqueue(action);
threadCanWorkHandler.Set();
return;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
return;

var isSuccessullyRemoved = taskQueue.TryDequeue(out Action? newAction);
if (isSuccessullyRemoved)
{
Interlocked.Add(ref amountOfWorkingThreads, 1);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Interlocked.Increment

if (!myThreadPool.cancellationTokenSource.IsCancellationRequested)
{
var nextFunction = new Func<TContinuationResult>(() => continueFunction(Result()));
return new MyTask<TContinuationResult>(myThreadPool, nextFunction);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Это сразу ставит в пул задачу-continuation, хотя она не может стартовать, пока первая задача не закончится. Получается, что если мы, скажем, поставим в пул задачу на 2 часа, потом 100 continuation-ов к ней на 1 секунду, то один поток из пула будет делать первую задачу, остальные потоки стоять на 165 строчке и не мочь делать другие, готовые к исполнению, задачи. Тут надо хитрее — если задача готова, ставить на пул, иначе складывать во временную очередь, и по готовности добавлять на пул (но подумать, что будет, если пул остановлен).

return new MyTask<TContinuationResult>(myThreadPool, nextFunction);
}
}
throw new InvalidOperationException("Shutdown was requested, threadpool stoped calculating");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
throw new InvalidOperationException("Shutdown was requested, threadpool stoped calculating");
throw new InvalidOperationException("Shutdown was requested, threadpool stopped calculating");

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.

2 participants