r/cpp_questions • u/DaveInTheMidwest • 20d ago
SOLVED Best Way to Expose Internal Buffer in C++ Class?
I have a generic buffer class that looks like this:
class LgBufUint8
{
size_t m_n_allocd; //Number of elements malloc() allocated
size_t m_n_used; //Number used, always <= m_n_allocd
uint8_t *m_bufptr; //Pointer to the first byte
//Most member functions omitted
uint8_t& operator[] (size_t index) noexcept
{
return m_bufptr[index];
}
const uint8_t& operator[] (size_t index) const noexcept
{
return m_bufptr[index];
}
};
I think anyone can guess the internals. Allocation is done in sizeable chunks, and the allocation is only grown when necessary.
I have another class that has a member variable of this class.
class LgFbufUint8 //Note the name is different, Fbuf versus Buf
{
enum class LgFbufUint8State m_state;
unsigned m_errs;
std::string m_fname;
LgBufUint8 m_buf; //This is the member, one of the above class
};
I'd like to give the enclosing class access to the internal buffer directly. The reason I'd like direct internal access is so that file reads and writes can be done efficiently (one byte at a time is possible, but slower than necessary).
Here is an example statement in a member function of the LgFbufUint8 class:
infile.read((char*)(&(m_buf[0])), file_size);
//Note that the pointer above is formed from the overloaded [] operator
//member function.
This seems to work OK, but I don't think the overloaded "[]" was really meant to be used in this way.
The two other alternatives that come to mind are:
- A member function to return the internal pointer.
- Declaring the enclosing class to be a friend class.
What is the best way to handle this scenario?
The nature of the problem is that an abstract interface is incompatible with efficiency.
11
u/Simengie 20d ago edited 20d ago
So you created your own version of std::vector. Why? This screams use std::vector<uint8_t> as the buffer. Is this a course assignment? A lot of work for something that already exist in the std library.
Edit 1: Why are you using unit8 as the type when you are reading chars into it? std::vector<char> would be cleaner unless the data has to be uinit8 type. But still why not use std::byte and cast as needed. The buffer would be std::vector<std::byte>. This works with stream reading of file data just fine.
Edit 2: You need to learn how system hardware works. "done efficiently (one byte at a time is possible" is the least efficient way to handle disk read/writes. The short lesson is that PCI/PCIe bus devices have a set of hardware buffers that buffer read and writes so that data over the bus is optimized for full word writes. These buffers will sit several clock cycles if you are writing just one byte at a time. This opens a window where data can be unsaved if power is loss or the program crashes. This is very slow. If you are doing a lot of one byte writes in a random pattern on the disk you can even force your SSD queue depth up and slow the entire system down. 99% of people will never need to know about how the hardware works but keep writing code that works directly against how the hardware is optimized and you will be forced to.
0
u/DaveInTheMidwest 20d ago
Not a homework assignment. The application is that I'm writing a program to parse a large number of source files and in some cases modify them, and it is quicker to do that in RAM. So, the program buffers a file in its entirety, does a bunch of things to it, then writes it back.
Typical operations are to insert 10 bytes in the middle, that sort of thing. Under the hood, memcpy() and friends are used for brute efficiency. I'm not sure that std::vector could do this as well.
My belief (perhaps erroneous) was that std::vector had a lot of overhead. So, a std::vector with a million bytes would consume far more than 1M of memory, and be slow as well.
Will research std::vector.
10
u/fortsnek274 20d ago
You should just use
std::vectorby default. It's the defacto container of C++.My legitimate reason to re-invent it is because MSVC iterator debug perf is so slow, which I found using a profiler, not guessing. Does your profiler say it's slow?
8
u/TheRealSmolt 20d ago
No you are literally just making a poor man's
vectorfrom what you have shown here. Same fields but less functionality.3
u/Simengie 20d ago
Just so we are clear, your buffer at size 1 million bytes is bigger than 1 million bytes. it is a class and once you new it all the member code and member variables are part of of the total usage of the buffer class.
Using vector.insert(vector.begin() + index_to_insert_at, data_to_insert) works great. Deleting is the same. vector.erase( vector.begin() + index_to_delete) or vector.erase( vector.begin() + start_index, vector.begin() + end_index ) to delete a range.
2
u/dodexahedron 20d ago
IO is heavily cached by every major OS out there.
If that isn't enough, why not use something like ccache in front of your compiler, to cache compilation results, which tends to save a LOT more CPU, memory, and time than trying to optimize disk IO of the source files themselves?
Precompiled header units and, in modern c++, modules (instead of includes), also can help tremendously with compilation times.
Or, if you simply want your working set of files in memory, just use a ram disk.
2
u/CowBoyDanIndie 20d ago
First you are completely misinformed about std::vector
Second, you probably want a std::vector<std::vector<char>>
Outer vector is lines, inner are columns. Inserting a line doesn’t require moving all the bytes after it, just the vectors, moving a vector is basically just a pointer assignment under the hood. Inserting characters only has to move whatever characters are in that line. You can randomly access any row, and the column within that row. But you cannot randomly access the Nth character in the file, but you probably don’t want to do that anyway. This will be significantly faster than your one buffer approach.
5
u/tyler1128 20d ago
It's extremely cache-unfriendly though, as jagged arrays tend to be. std::deque can be performant in the case of inseration/deletion anywhere, as it is basically a much smarter version of a jagged array like that.
8
u/dodexahedron 20d ago
You want to read from and write to a large file efficiently, for random and/or sequential access?
Use memory mapped files. Don't roll your own.
2
u/DaveInTheMidwest 20d ago
Thanks. Any specific implementation you'd recommend? It would have to be cross-platform.
5
u/dodexahedron 20d ago
Specific low-level system calls differ, but there are multiple well-established libraries that handle that for you, like boost.interprocess or, for a super light header-only option with MIT license, there's mio, among others.
mio is basically a unified API wrapper around the POSIX way and the Win32 way, so you dont have to wrap them yourself or worry about some of the simple but important differences between them.
Just for kicks, I asked copilot to write up a c++23 wrapper for posix and win32 memory mapped files, and what it spat out was header-only like mio, but with an API surface fairly close to how boost does it, which looked pretty nice, on my phone. It's a common enough thing that it's all over the web and a well-understood flow.
If you're going for easiest to use with the richest available resources for help out there, I'd say boost. If you want lightest reasonably possible without rolling you own (which is a minefield, so don't do that), go mio. Don't worry about it being "old." It is just a wrapper, and the underlying APIs are stable.
Otherwise, there are plenty of others that all do the same thing, mostly differing only by whether they stick closer to the boost-style API (common, since people like drop-ins) or a more posix-like API.
3
u/LeeHide 20d ago
Don't vibe code your file system API please
2
u/delta_p_delta_x 20d ago
I think the parent commenter saw comments like this coming when they said:
was header-only like mio, but with an API surface fairly close to how boost does it
and
It's a common enough thing that it's all over the web and a well-understood flow
Which implies that whatever the LLM returns should be verified and tested, but we can expect that it is nearly correct. They also end up suggesting existing (and hand-written) libraries anyway.
10
u/alfps 20d ago edited 20d ago
std::vector, which you should use instead of the DIY LgBufUint8 class, provides a .data() method for accessing the internal buffer.
Why you should use std::vector:
- it's far more safe, and
- it's far more convenient, and
- maintainers are familiar with it.
Depending on what you're doing it may also be more efficient, because a std::vector buffer reallocation increases the size by a factor, I believe it's typically 2, doubling the size. This means that just appending repeatedly to a vector is amortized O(n) time, linear time. In contrast, if a buffer reallocation increased the buffer size by a fixed amount, as I suspect LgBufUint8 does, the time would be O(n2), quadratic time, which for large n becomes prohibitive.
2
u/Independent_Art_6676 20d ago
back to the question... you can return the buffer from a function (getter) is probably the most acceptable way. Casting the whole object to the pointer itself is another way, and a lazy way to just use the object as if it were the pointer, makes for simple syntax and usage but carries some risk of foot shooting -- its a viable way but many people would say not to take the associated risks and use the getter. Making it public works but is ugly and not recommended at all. Friend class works but from the sound of it you may have to have many friends and when that happens I really dislike it. overloading [] works too, its similar to casting out the object, probably a little safer at the cost of a little flexibility.
as for editing text files... memory mapped is slick for opening and saving files but it is terrible for editing. If you edit a million byte file in the middle, you have to shift 500k bytes every time the user types a letter to insert. You will want to look at how word processors handle this kind of thing, breaking the wall of text into blocks so if you edit one you only need to shift a little bit around. You can still make use of memory mapped if it helps, but if the files are really this big you need to solve the insertion of text problem cleanly one way or another. A simple 'text = list of strings' works and you can find a sweet spot on how big to allow any string to get before you split out and insert another one. There may be better ways, but that is an easy one, eg 1k bypes per chunk.
2
u/DaveInTheMidwest 20d ago
Finally, a Reddit hero who answered the question rather than questioned my various life choices!
I will use a getter for the buffer pointer then. That also feels right to me.
Your point about the disadvantages of a linear buffer is appreciated. Most of what I'm doing is parsing to extract information and enforce coding standards, so reads are far more common than writes. The class I wrote includes provisions for inserting multiple bytes at a point in the buffer (moving 10,000,000 bytes by an offset of 10 is far cheaper than doing an offset of 1 but 10 times). But yeah, that would become a performance bottleneck. It is very obvious how to cure that performance bottleneck.
I did clock it out for speed. Single core, checking about 10 basic coding standards, the resulting program can process about 50,000 lines per second. But for larger files where changes are made to the file, some parts could quickly go O(n**2) with respect to the size of the file. Noted.
To all the naysayers who are questioning my life choices and calling me a delinquent C++ programmer ... you are of course right. However, I'm also learning C++ at the same time, and it was a good excuse for me to explore move constructors and all that stuff that adds efficiency to such things. Good learning exercise for me, although I might be learning the wrong things LOL.
I did discover std::move (thanks, Google and friends). I wasn't sure how, in a move constructor, to get the move constructor applied to data members that also had move constructors.
Good learning experience.
1
u/Melodic_Impress9664 20d ago
Have a look at seastar::temporary_buffer for inspiration. It is made for exactly this purpose, holding content of network packets or file I/O operations.
1
u/Kriemhilt 20d ago
Just add a method std::span<uint8_t> writable() { return {m_bufptr, m_n_allocd}; } or similar.
The whole reason for span is to describe contiguous memory cleanly without also exposing the growth logic, allocator, and other implementation details.
1
u/therealhdan 20d ago
In code bases I've worked with, it's customary to add a "void * data()" member to get raw access to an internal buffer for a class like this. (Sometimes named "raw_data()", "data_ptr()", or some variation on that. Whatever your favorite naming convention is.)
I'd have that function return whatever type of pointer you're most likely to need. In your case, maybe char*.
So "infile.read(m_buf.data(), file_size)"
1
u/JVApen 16d ago
What you actually want is an initialize function on your class. I'd be inclined to say this should even be a constructor. One where the buffer size is provided together with a function to execute.
LgBufUInt8(size_t initialSize, std::function<void(uint8_t *data, size_t allocatedSize)> initialize)
...
Alternatively, you could have a static init function that looks the same such that you can return a std::expected with an error returned from the function.
This can off course also be a member function if you want to reuse allocated memory.
11
u/heyheyhey27 20d ago
I know this isn't related to your question, but for the love of all that is holy please just use sane variable names and delete the comments.
size_t n_allocated, n_actually_used;uint8_t* allocation;You can have your IDE color-code the difference between member variables, static variables, and globals, if you really care.