HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Reading a pointer to a string

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pointerreadingstring

Problem

What it is doing basically is reading an std::string from a given pointer.

I was told that using a StringBuilder as I am is not the best way to achieve this, so I would like to know how this would be best written.

As additional information, that is a pointer of a std::string and I've made that because I was originally curious if there was a way to read a std::string in C# without having to create a bridge using C++/CLI.

How could I optimize this bit of code for best performance?

public static string ReadStdString(IntPtr address)
{
    var capacity = Marshal.ReadInt32(address + 24);
    var length = Marshal.ReadInt32(address + 20);
    var startStr = address + 4;
    if (capacity > 15)
        startStr = (IntPtr)Marshal.ReadInt32(address + 4);

    var result = new StringBuilder();
    for (var i = 0; i < length; i++)
    {
        result.Append((char)Marshal.ReadByte(startStr, i));
    }
    return result.ToString();
}

Solution

var result = new StringBuilder();
for (var i = 0; i < length; i++)
{
    result.Append((char)Marshal.ReadByte(startStr, i));
}


You're working in a tight loop: a StringBuilder looks like a reasonable tool to use.

One thing I would change that could impact performance (depending on the length of the string involved), is the StringBuilder constructor being used:

var result = new StringBuilder(length);


There's no reason not to specify the length of the string you're building if you know it from the start; that will reduce the overhead, since the internals of the builder won't need to manage growth.

The loop is a one-liner. You have this one-liner just a few instructions above:

if (capacity > 15)
    startStr = (IntPtr)Marshal.ReadInt32(address + 4);


Why does the loop have an explicit { } scope, but not the if block? It would be better to be consistent about scoping braces, and have them everywhere:

if (capacity > 15)
 {
     startStr = (IntPtr)Marshal.ReadInt32(address + 4);
 }


An alternative to the StringBuilder could be to write the bytes into a MemoryStream:

using (var stream = new MemoryStream(length))
{
    for (var i = 0; i < length; i++)
    {
        stream.WriteByte(Marshal.ReadByte(startStr, i));
    }
    return new StreamReader(stream).ReadToEnd();
}


The nice thing with this approach is that you don't need to cast every single byte of the string into a char. On the other hand, you need 2 objects instead of one, and you need to cleanly Dispose of the stream, too - depending on the length of the string, the overhead might just not be worth it, although my guts tell me the stream could be faster... but I think you'd need to race your horses to find out.

Code Snippets

var result = new StringBuilder();
for (var i = 0; i < length; i++)
{
    result.Append((char)Marshal.ReadByte(startStr, i));
}
var result = new StringBuilder(length);
if (capacity > 15)
    startStr = (IntPtr)Marshal.ReadInt32(address + 4);
if (capacity > 15)
 {
     startStr = (IntPtr)Marshal.ReadInt32(address + 4);
 }
using (var stream = new MemoryStream(length))
{
    for (var i = 0; i < length; i++)
    {
        stream.WriteByte(Marshal.ReadByte(startStr, i));
    }
    return new StreamReader(stream).ReadToEnd();
}

Context

StackExchange Code Review Q#97186, answer score: 4

Revisions (0)

No revisions yet.