Why get address of variable is unsafe ?

[MBurtsev]

When I use operator for get address(&) compiler shows necessary using unsafe context. But if we just get address and never use it as pointer we are always in safe context. I suggest making changes. So that this requirement applies only if pointers are used for read\write operations. Below I give the code that should work in a safe context.

        static void Test()
        {
            var adr = 0;
            var threadId = (long)&adr >> 16;
        }

Edit:
Another solution is to add an operator like __makeref. For example __stack which will return the top of the call stack(rsp register).

[MBurtsev]

I made two benchmarks:

With ManagedThreadId. class, bench

Method	Threads	Mean	Error	StdDev	Min	Max	Op/s	Op/s total
Write	1	11.25 ns	2.003 ns	0.520 ns	10.45 ns	11.78 ns	88,884,451.8	88,884,451.80
Write	2	11.88 ns	1.340 ns	0.348 ns	11.49 ns	12.21 ns	84,161,901.6	168,323,803.20
Write	4	12.37 ns	1.732 ns	0.268 ns	11.97 ns	12.54 ns	80,847,692.9	323,390,771.60
Write	8	15.55 ns	3.478 ns	0.538 ns	14.77 ns	15.97 ns	64,320,056.6	514,560,452.80

Without class, bench

Method	Threads	Mean	Error	StdDev	Min	Max	Op/s	Op/s total
Write	1	6.903 ns	0.3176 ns	0.0491 ns	6.845 ns	6.964 ns	144,873,381.9	144,873,381.90
Write	2	7.493 ns	0.2879 ns	0.0748 ns	7.392 ns	7.586 ns	133,464,121.1	266,928,242.20
Write	4	7.950 ns	0.4700 ns	0.1221 ns	7.736 ns	8.033 ns	125,788,900.8	503,155,603.20
Write	8	11.269 ns	3.0206 ns	0.4674 ns	10.685 ns	11.778 ns	88,738,969.3	709,911,754.40

[yaakov-h]

How about Environment.CurrentManagedThreadId instead?

[MBurtsev]

    public partial class Environment
    {
        public static int CurrentManagedThreadId => Thread.CurrentThread.ManagedThreadId;

[yaakov-h]

namespace System
{
    public static partial class Environment
    {
        public static extern int CurrentManagedThreadId
        {
            [MethodImpl(MethodImplOptions.InternalCall)]
            get;
        }

https://source.dot.net/#System.Private.CoreLib/src/System/Environment.CoreCLR.cs,0b1681fc8a648379

[MBurtsev]

I found it here

With Environment.CurrentManagedThreadId

Method	Threads	Mean	Error	StdDev	Min	Max	Op/s	Op/s total
Write	1	11.03 ns	3.010 ns	0.782 ns	10.21 ns	12.23 ns	90,690,349.1	90,690,349.10
Write	2	11.66 ns	2.826 ns	0.734 ns	10.79 ns	12.70 ns	85,730,947.9	171,461,895.80
Write	4	11.94 ns	1.546 ns	0.402 ns	11.46 ns	12.51 ns	83,751,769.7	335,007,078.80
Write	8	15.35 ns	2.392 ns	0.621 ns	14.51 ns	15.96 ns	65,156,907.3	521,255,258.40

With Thread.CurrentThread.ManagedThreadId

Method	Threads	Mean	Error	StdDev	Min	Max	Op/s	Op/s total
Write	1	10.56 ns	4.182 ns	0.647 ns	9.599 ns	11.00 ns	94,690,726.5	94,690,726.50
Write	2	10.91 ns	1.902 ns	0.294 ns	10.541 ns	11.23 ns	91,699,381.2	183,398,762.40
Write	4	11.65 ns	1.922 ns	0.297 ns	11.432 ns	12.09 ns	85,851,836.3	343,407,345.20
Write	8	15.02 ns	2.330 ns	0.605 ns	14.155 ns	15.66 ns	66,561,493.3	532,491,946.40

With stack

Method	Threads	Mean	Error	StdDev	Min	Max	Op/s	Op/s total
Write	1	6.978 ns	0.1525 ns	0.0396 ns	6.945 ns	7.038 ns	143,316,919.9	143,316,919.90
Write	2	7.423 ns	0.3070 ns	0.0475 ns	7.354 ns	7.460 ns	134,711,222.6	269,422,445.20
Write	4	8.151 ns	0.1704 ns	0.0443 ns	8.106 ns	8.212 ns	122,677,419.0	490,709,676.00
Write	8	11.772 ns	1.9715 ns	0.5120 ns	10.942 ns	12.316 ns	84,947,409.1	679,579,272.80

Sources

[bernd5]

Can you explain what do you want with that pointer?
The pointer value to the local variable might be always the same... (depending on your current stack)

[MBurtsev]

do you want with that pointer?

Nothing with pointers. But getting the address of a variable in memory can be useful.

might be always the same...

This can be used to get the thread id. This is useful for making highly efficient algorithms with bounded memory ways.

        public unsafe void Write(T value)
        {
            unchecked
            {
                var adr = 0;
                var channel = _channel;
                var id      = (int)((long)&adr >> 16);
                var buffer  = channel.WriterLinks[id % channel.Size];

                if (buffer.Id != id)
                {
                    buffer = SetupThread(id);
                }

                var pos = buffer.WriterPosition;

                if (pos == SEGMENT_CAPACITY)
                {
                    SetNextSegment(channel, id, value);

                    return;
                }

                buffer.WriterMessages[pos] = value;
                buffer.WriterPosition = pos + 1;
            }
        }

[svick]

I don't think this is sufficient justification to change the language, which is something that requires fairly significant effort, even for a small change like this one.

This change would bring a tiny benefit (not requiring to use unsafe) to a tiny number of people (since your use case seems to be extremely narrow). Generally, language changes need to offer significant benefit or affect significant number of people.

And your use case still feels fairly dangerous to me: what guarantees that stacks of two threads can't occupy the same 64 kB block of memory? So I'm not sure not requiring unsafe would even be a positive change.

[theunrepentantgeek]

How can a local Int32 variable get into the heap? The heap can only contain objects that have references.

I don't want to divert this thread with oodles of example code, so here are two pointers for further research.

• If you have a method decorated with async then the body of the method is rewritten by the compiler into a state machine that coordinates the asynchrony, an instance of which lives on the heap. All local variables used by the asynchronous code are lifted onto the class implementing that state machine, and therefore live in the heap.
• If your method contains a lambda expression, any local variables consumed by the body of that lambda expression will similarly be lifted onto a heap dwelling instance; this is required to allow the lambda expression to have a longer lifetime than the original method.

There are also novel implementations of .NET that run without a stack at all - one of those is HastLayer, for example.

[MBurtsev]

@theunrepentantgeek

I cant use await in unsafe context

[MBurtsev]

In any case, if you're not doing read/write operations then getting the address is safe even if it's on the heap.

[CyrusNajmabadi]

It's not safe as a pointer 'address' of is not a safe C# concept.

[CyrusNajmabadi]

I cant use await in unsafe context

Right. And we would not make this start working here.

[CyrusNajmabadi]

What's the value in getting the address of something that you don't then intend to read/write. I don't understand the use case.

[CyrusNajmabadi]

We need to develop not the platform, but the compiler. We will only declare our intentions, the compiler will do the rest.

@MBurtsev this is the repo for the c# language, not the compiler. Requests for compiler changes should be made at dotnet/Roslyn (for the c# compiler) or at dotnet/runtime (for the jit,/runtime/aot compilers).

However, what your are asking for in your OP is a change to the language. Something which you have not only not justified, but where you seem to be ignoring what multiple lang designers are saying. You are then continuing to act as if you are blocked from getting the performance you want.

You are not blocked. Your are allowed to do this operation. You simply just declare your understanding that this is not safe and you can do it. From my perspective, this is ideal. You get the performance you need, but you understand that it's might break at any point in the future.

[MBurtsev]

Friends, I have already come to terms with the fact that I will use unsafe. Thank you all for discussing this issue.

[CyrusNajmabadi]

@MBurtsev Your answers are coming across as extremely sarcastic and antagonistic. For example:

I agree that when the dark elves come and make their PC without a stack, so be it. And also when the processor will not have registers, and when the variables in memory will not have addresses

Please tone it down. This is the repo for the language. It is designed understanding that for now interesting changes have already happened numerous times, so the language should not make assumptions that tie it tightly to one particular impl that behaves one particular way.

[tannergooding]

Your advice is 300% slower.

You're effectively holding it wrong here. You generally wouldn't want to have class ChannelData that itself contains multiple ThreadLocal<T>. Instead you typically have a single ThreadLocal<U> that contains your necessary per-thread data, you'd likewise generally make the ThreadLocal<U> data static and have it expose the per-thread data at the highest level.

This makes a non-trivial difference in whether you have repeated or one-time lookups. The need to get this "per-thread" data is explicitly meant to be "hoisted" as high as possible to help reduce the overhead and cost. This applies no matter which way you decided to have "per-thread data", even if it was simply some Dictionary<int, U>.

Likewise benchmarking multi-threaded code is also very error prone in the first place. Micro-benchmarks run the same code repeatedly in a loop and therefore tend to hit the "best case scenario" for the CPU data, instruction, and microcode caches. This means that it is often "trivial" to write some simple algorithm that appears to perform better than what's built into .NET because the algorithm is effectively only measuring "best case" and is basically tuned explicitly for looking good in a micro-benchmark. The .NET types and code, including Thread Local Storage are explicitly written and tuned for real world apps. The best, worst, and average case scenarios are well understood across a range of hardware (from small 1-core, to big many core server systems) and in how the behave in both light, medium, and heavy contention.

[MBurtsev]

@tannergooding

you'd likewise generally make the ThreadLocal data static

How will a static field work with multiple class instances?

You're effectively holding it wrong here. You generally wouldn't want to have class ChannelData

Here is TryRead method. For it needs two fields ChannelData.Head and ChannelData.Reader. And there is no need for others including ThreadLocal. There is also a Clear method, which should clear all data by writing a reference to a new instance with a single atomic operation. Such an organization with the help of ChannelData allows to separate the work of reading and writing threads. To reduce the negative effects of false sharing. Maybe I'm not destined to fully understand your idea without seeing the code...

Likewise benchmarking multi-threaded code is also very error prone in the first place.
The best, worst, and average case scenarios are well understood across a range of hardware

I don't believe what you wrote. I believe in the results I get when doing tests. This is the scientific approach. Any statement requires proof. Making statements that cannot be proven are not credible. If you have tests for exotic situations, please provide them, I will also check them. All I see now is some of your beliefs that you pass off as reality. I find it hard to believe without hard evidence that a method that works is 300% faster in the test. In another situation it will be worse. In the same situations, the fast method will work faster than the slow one.

[theunrepentantgeek]

The platform runs on existing hardware. The architecture will not change for the foreseeable future.

So? Language changes need to make sense for every existing platform, not just your narrow special case.

How do you cache it if it's the only way to split the threads. I am forced to use this in every operation. The main idea here is that each thread operates in its own area of memory. With this approach, you don't need CAS operations.

This sounds like an incredibly narrow special case. What's the value in the language changing for ALL users just to accommodate your esoteric application?

I'm not denying the value to you in your situation - but languages changes aren't made to suit a single case; they're tremendously expensive to implement and incur an ongoing testing and maintenance burden, so changes need to make sense for a wider audience.

[MBurtsev]

Because it scares a lot of people. And secondly, because simply getting an address is not unsafe :). So far, no one has shown a situation in which this would be dangerous.

[MBurtsev]

You can convince me. If you show a couple of lines of code, when you can crash the app without reading and writing. Or make it open to hacking.

[CyrusNajmabadi]

Because it scares a lot of people.

Why is that relevant? It's your code.

[HaloFour]

Because it scares a lot of people.

Unless they have to write the code what does it matter? It's not like you have to mark a method as unsafe in order to call an unsafe method.

And secondly, because simply getting an address is not unsafe :). So far, no one has shown a situation in which this would be dangerous.

The language isn't going to split hairs because you think your scenario is "safe enough". You touch a pointer you're in "unsafe" territory. There's no reason to expand the heuristics of the compiler to try to figure out if how you're treating that pointer may or may not actually lead to problems.

[MBurtsev]

Okay guys I saw your position.

[AartBluestoke]

@MBurtsev another way to get the same 'random' distribution is to have something like

public class RoundRobbin{

    private static int _current=0;
    private static int GetNext => System.Threading.Interlocked.Increment(ref _current);
    
    static void Test()
    {
        var threadId = GetNext % Environment.ProcessorCount;
    }
}

[MBurtsev]

Thanks for your reply. It's a nice idea, but unfortunately it's too slow. I have tested it before. The only reason here is that the code is not cache friendly. That's why it's very slow. Also, we cannot use ProcessorCount. This is not thread safe, see the example below.

    public class RoundRobbin
    {
        const int _count = 100_000_000;
        private int _threads = 4;
        private int _current = 0;
        private int _divider;
        private int[] _storage;

        public RoundRobbin()
        {
            // not thread safe
            //_divider = Environment.ProcessorCount;
            // thread safe. This number must be large enough to use the time lag effect.
            _divider = 1024;
            _storage = new int[_divider];
        }

        public static void Test()
        {
            var ready   = 0;
            var obj = new RoundRobbin();

            for (var n = 0; n < obj._threads; n++)
            {
                Task.Factory.StartNew(() =>
                {
                    for (int i = 0; i < _count; i++)
                    {
                        obj.Add();
                    }

                    if (Interlocked.Add(ref ready, 1) == obj._threads)
                    {
                        obj.Check();
                    }

                }, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default);
            }
        }

        public void Add()
        {
            var id = Interlocked.Increment(ref _current) % _divider;

            _storage[id]++;
        }

        public void Check()
        {
            var count = 0;

            for (var i = 0; i < _divider; ++i)
            {
                count += _storage[i];
            }

            if (count == _count * _threads)
            {
                Console.Write("It is thread safe");
            }
            else
            {
                Console.Write("It's not thread safe");
            }
        }
    }