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.

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

Comment thread Homework3.Tests/MyTreadPoolTests.cs Outdated
@@ -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, их тут тоже надо поправить. Не буду их повторять.

Comment thread Homework3.Tests/MyTreadPoolTests.cs Outdated
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.

Comment thread Homework3/IMyTask.cs
@@ -0,0 +1,24 @@
using static Homework3.MyThreadPool;

public interface IMyTask<T>

This comment was marked as resolved.

Comment thread Homework3/MyThreadPool.cs Outdated
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;

Comment thread Homework3/MyThreadPool.cs Outdated
{
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?

Comment thread Homework3/MyThreadPool.cs
}
if (Volatile.Read(ref exception) == null)
{
return result;

This comment was marked as resolved.

Comment thread Homework3/MyThreadPool.cs Outdated
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 это очень тяжело будет сделать правильно.

Comment thread Homework3/MyThreadPool.cs Outdated
{
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 thread Homework3/IMyTask.cs
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.

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

Comment thread Homework3/MyThreadPool.cs
{
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.

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

Comment thread Homework3/MyThreadPool.cs
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 thread Homework3/MyThreadPool.cs
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

Comment thread Homework3/MyThreadPool.cs
{
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;

Comment thread Homework3/MyThreadPool.cs
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

Comment thread Homework3/MyThreadPool.cs
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 строчке и не мочь делать другие, готовые к исполнению, задачи. Тут надо хитрее — если задача готова, ставить на пул, иначе складывать во временную очередь, и по готовности добавлять на пул (но подумать, что будет, если пул остановлен).

Comment thread Homework3/MyThreadPool.cs
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