KEMBAR78
Clean Near-Unmaintainable Parallel Logic · Issue #53593 · dotnet/runtime · GitHub
Skip to content

Clean Near-Unmaintainable Parallel Logic #53593

@Alex-ABPerson

Description

@Alex-ABPerson

The problem

The code within Parallel has remained extremely similar since its introduction all the way back in Framework. And over the years it has become increasingly less maintainable, to the point that someone who worked on it may have even left a comment on their difficulties working with it (see below).

There are only four methods that hold practically the entire behaviour of Parallel:

  • Invoke
  • ForWorker<TLocal>
  • ForWorker64<TLocal>
  • ForEachWorker<TSource, TLocal>

All of the features, thread locals, parallel loop state changes, passing indices to methods etc. are all performed by these four methods for every mode, and simply enabled or disabled by making arguments on them as null.

This is a perfectly fine idea if executed properly, but as it is the methods are quite a mess. Almost all of it has been packed into a single, huge, method, for all four modes. There's no structuring, none of it has been split up and organised into smaller sub-methods, there's loads of seemingly duplicated logic across all four modes, it's all just become one big slew of logic from start to finish.

It's possible the issue is bad enough that a developer has even left a comment about it:

This comment may be talking from the code's perspective saying that "this task won't maintain this value" or something along those lines, or it may literally be talking from the developer's perspective saying that they're just going to do this because they're not really sure of the exact implications, which given the state of the method isn't an unreasonable possibility. It's not entirely clear which it is without untangling the logic, which I won't do if you decide it's not worthwhile to clean these up.

Why it matters

Overall, this does make it very hard to maintain the Parallel methods and it's only going to get worse. When it comes around to looking into issues like #50566 as the code currently is it may be a painful process if it's needed to look through or even debug these methods and attempt to apply a fix.

In addition, the way the methods are packed in currently makes it harder to spot any sort of redundant logic, hindering possible performance opportunities too.

If it is decided that it is a worthwhile process to try to clean it up, try to remove any repeated code, refactor it into smaller sub-methods etc. then I'd happily take it up myself.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions