ETLCPP / etl

Embedded Template Library
https://www.etlcpp.com
MIT License
2.21k stars 392 forks source link

how to use an ETL container on an "external" memory bank #375

Closed jerabaul29 closed 3 years ago

jerabaul29 commented 3 years ago

I am using some nice non volatile FRAM chips (my specific modules are https://www.adafruit.com/product/4719 from Adafruit, but I guess the exact model makes no difference). These are nice, big banks of "external" non volatile memory.

I would like to have some ETL containers living fully (i.e., both the data buffer, and the "container magics" around them) on such an external memory bank. These have a very simple API, something like:

fram.write8(address, byte_value);
fram.read8(address);

This allows to write and read individual bytes of data at individual addresses easily.

Any way / how to make an ETL container live on such an external memory bank with an API to access it? I looked at the documentation but did not find hints of how I could do something like this.

jwellbelove commented 3 years ago

etl::vector and etl::deque can use external memory blocks to store data, but they expect it to operate as normal random access memory. There is no facility to access the memory block via an API. For the containers to be non-volatile, all of the member variables of the container would have to be situated in the FRAM to ensure that container started in the correct state. You would need to create your own containers to interface with this FRAM.

jerabaul29 commented 3 years ago

Yes, agree about the all member variables on FRAM point - this is what I meant, you write it much better.

Ok, so if I understand well you mean that this is outside the scope of ETL more or less, right?

jwellbelove commented 3 years ago

It would be quite a lot of work to create non-volatile versions of all of the containers in the ETL. The current ones could not easily be adapted without seriously affecting their performance for use with normal RAM.

jerabaul29 commented 3 years ago

I understand and that is very fair 😊 Closing as out of the scope of ETL, thanks for the discussion!

jerabaul29 commented 3 years ago

I have been thinking about this some more as this is a feature that would be really convenient to have when working with some data that really needs to survive reboots / interruptions etc. I have some boards that specifically have some non volatile memory banks just for this purpose, feels very sad to not use these.

I was thinking that maybe a weaker form of this would be possible and still nearly as useful. My idea is the following: could we have a couple of functions, defined on any container, that:

size_t complete_memory_footprint(etl::container const & container_in)
using MemoryDumper = void (*)(size_t const byte_index, unsigned char const to_write);
void dump_container(etl::container const & container_in, MemoryDumper memory_dumper);
using MemoryReader = unsigned char (*) (size_t const byte_index);
void restore_container(etl::container & container_out, MemoryReader memory_reader);

Do you think this could fly? Any weakness you see here? Some stuff that could be made in a smarter way?

I would really need some functionality like this, so 1) if you think this could fly and may be a good idea 2) if you help me decide on the API, 3) if you would be willing to integrate this into ETL, I will be willing to put a bit of time in this and try implement something :) .

jerabaul29 commented 3 years ago

Also, how general vs tuned to each container do you think this kind of approach could be? :) . For my personal use, etl::vector and etl::deque would be enough, and I guess I could implement these by hand for each of these cases if I want, but curious if you think there is a smart way to do that in a general way?

jerabaul29 commented 3 years ago

This would enable a workflow like:

Wonder if there could be an additional byte of memory used to keep track of whether this has ever been used to dump, so that we know at reboot if there is actual data to grab (another possibility there would be that the user runs a code before uploading the code needing this feature, to "pre-initialize" the containers to an empty state before the first boot of the program needing these features).

jwellbelove commented 3 years ago

That's some interesting ideas. Food for thought.

jerabaul29 commented 3 years ago

Great you find it interesting :) .

Looking forward to hearing your thoughts then :) .

jwellbelove commented 3 years ago

Another way could be to give the container an instance of a type derived from a 'persistence' interface. This could possibly be made very generic, to enable that functionality to be easily added to other types.

Something along the lines of:-

namespace etl
{
    // Persistence interface.
    class ipersistence
    {
    public:

        virtual void start() = 0;

        virtual void save(const char* data, size_t length) = 0;
        virtual void save(char c) = 0;

        virtual void restore(char* data, size_t length) = 0;
        virtual void restore(char& c) = 0;
    };
}

// Derived version that persists to NVRAM.
class NVRam : public etl::ipersistence
{
public:

    NVRam()
      : index(0U)
    {
    }

    void start() override
    {
        index = 0U;
    }

    void save(const char* data, size_t length) override
    {
        NVRamWriteBlock(index, *data, length);
        index += length;
    }

    void save(char c) override
    {
        NVRamWrite(index, c);
        ++index;
    }

    void restore(char* data, size_t length) override
    {
        NVRamReadBlock(index, *data, length);
        index += length;
    }

    void restore(char& c) override
    {
        NVRamRead(index, c);
        ++index;
    }

private:

    size_t start_index;
    size_t index;
};

// Derived version that persists to a file.
class File : public etl::persistence
{
    //...
};

etl::vector<int, 10> v;
NVRam nvram;
File  file;

v.save_to(nvram);
v.restore_from(file);
jerabaul29 commented 3 years ago

In this case the user would have the responsibility to implement the NVRam / File classes by defining the virtual methods of the base ipersistence class, right? That sounds very good to me, looks very nice and clean.

I suppose also that it would be possible to use save_to and restore_from both from either a the nvram or the file, and that you show both of them just to illustrate that different non volatile "backends" could be used, right? :)

How much work do you think it would be to implement the save_to and restore_from methods on the etl containers? Is it just a matter of taking a sizeof on the templated class to know how much memory is needed, and use the pointer to selv to get to the start of the memory footprint? Or is there more work?

jwellbelove commented 3 years ago

Yes, the idea is that the container knows nothing about how the data is persisted, just that it will be given an object that implements the interface, allowing any number of persistence types to be defined.

The persistence would probably have to be limited to trivially constructible and copyable types.

I don't think the data for the container could always be just copied from sizeof and this. etl::vector for instance calculates its size from the begin and end pointers. The persistence data would just store a size value and data items and reconstruct the pointers from them in the restore_from.

etl::ivector would probably look like this.

  template <typename T>
  class ivector
  {
      void save_to(etl::persistence &p)
      {
          // Save the size.
          size_t vector_size = size();
          const char* pdata = reinterpret_cast<const char*>(&vector_size);
          size_t length = sizeof(vector_size);
          p.save(pdata, length);

          // Save the data.
          pdata      = reinterpret_cast<const char*>(p_buffer);
          length = size() * sizeof(T);
          p.save(pdata, length);
      }

      void restore_from(etl::persistence &p)
      {
          // Restore the size.
          size_t vector_size;
          char* pdata = reinterpret_cast<char*>(&vector_size);
          size_t length = sizeof(vector_size);
          p.restore(pdata, length);

          // Restore the data.
          pdata  = reinterpret_cast<char*>(p_buffer);
          length = vector_size * sizeof(T);
          p.restore(pdata, length);

          // Fix the end pointer.
          p_end = p_buffer + vector_size;
      }
  };
jwellbelove commented 3 years ago

There is another possible way to implement this that allows non-trivial types to be persisted. The etl::ipersistence class remains, but the containers call overloaded etl::save_persistent & etl::restore_persistent functions. By default, overloads for fundamental and ETL types are defined. The user defines overloads for their own types. This should also allow moveable types.

jerabaul29 commented 3 years ago

Ok. That makes sense, but I will have to look in the implementation in more details to make sure I understand everything :)

However, I wonder if it would be better to have a fixed dump size based on capacity rather than size. This way we could plan 'statically' for the memory layout in the non volatile memory bank. I think that would be much more convenient if one wants to dump several etl containers in a FRAM bank for example. This is why I was thinking about just dumping the full container - 'metadata', full memory (both in use and not in use), so that the size of the dump is always constant, and related to the container kind, template type, and capacity, independently of the size at the time of dumping.

jwellbelove commented 3 years ago

Yes, I see your point. I'll give that some thought. It's good to thrash these ideas out before committing to any solution.

jerabaul29 commented 3 years ago

I think I am just a bit confused about what lives where. I guess at least the 'container metadata' should be fully 'registered' in the call to sizeof on a container instance, right? But if the actual memory containing the array lives out of the stack and is statically allocated in the data section or something like this (guess this is what is done?), I see what you mean that the total size of it needs to be computed as you show.

Should / could a solution then be to 1) use sizeof on the container to get the size of the metadata and make sure that all of it is captured / dumped / restored, 2) compute the sizes as you point out but based on the total capacity to make sure we copy the data array completely when copying back and forth? :)

jerabaul29 commented 3 years ago

This also raises the question: does a container <typename T, size_t size> has a (static / constexpr so that it is free) notion of how much memory space its data array takes? That would spare us the task of calculating it by hand here and there :) .

jerabaul29 commented 3 years ago

It's good to thrash these ideas out before committing to any solution.

Agree, it is really nice to discuss here :) .

jwellbelove commented 3 years ago

I've created a feature branch to experiment with a persistence interface. feature/persistence There is a new file include/etl/experimental/persistence.h It defines the following:-


class ipersistence The user must supply a class derived from this to interface with the persistent storage. Defines the following pure virtual functions:-

void start()
    Resets the index to the start of the persistent storage.

void step(size_t n)
    Steps 'n' char along the persistent storage.

void save(const char* data, size_t length)
    Saves 'length' char starting from the address 'data'.

void load(char* data, size_t length)
    Loads 'length' char to the location starting at address 'data'.

virtual void flush()
    Flushes the persistent data to the store (if applicable).

class persistence_profiler A class derived from etl::ipersistence A dummy persistence class that calculates the require storage size.


void save_to_persistent(etl::experimental::ipersistence& persistence, T) A generic save function for integrals, floating point and pointers.

etl::experimental::ipersistence& operator <<(etl::experimental::ipersistence& ip, T value) A generic save streaming function for integrals, floating point and pointers.


void load_from_persistent(etl::experimental::ipersistence& persistence, T& value) A generic load function for integrals, floating point and pointers.

etl::experimental::ipersistence& operator >>(etl::experimental::ipersistence& ip, T& value) A generic load streaming function for integrals, floating point and pointers.


size_t persistence_size(const T& value) Calculates the storage size for persisting a T. Before C++11

size_t persistence_size(T&& value) Calculates the storage size for persisting a T. From C++11


void step_persistent(etl::experimental::ipersistence& persistence, const T& value) Steps persistence_size(T) char along the persistent storage. Before C++11

void step_persistent(etl::experimental::ipersistence& persistence, T&& value) Steps persistence_size(T) char along the persistent storage. From C++11


The following have been defined for etl::basic_string and etl::array

    save_to_persistent
    load_from_persistent
Streaming operators

For all other types the user must define:-

    save_to_persistent
    load_from_persistent
Streaming operators

There is a unit test defined that contains simple examples. test_persistence.cpp

jerabaul29 commented 3 years ago

That looks amazing, will find time next week to look at this in details :) .

jwellbelove commented 3 years ago

What it can't cope with are types that are only moveable or have no default constructor..

jwellbelove commented 3 years ago

Moveable types could maybe be supported if a reference to an uninitialised memory block was passed to load_persistent and the value was placement new'd into it. The issue would be differentiating between uninitialised blocks and actual constructed types, as one would need to be destructed first and the other not.

jwellbelove commented 3 years ago

@jerabaul29 Did you get a chance to look at it?

daniel-brosche commented 3 years ago

etl::vector and etl::deque can use external memory blocks to store data, but they expect it to operate as normal random access memory

I'm working on an embedded project that now has the need to dynamicaly specify the size of etl containers during the application startup. Currently all container sizes are specified as template argument SIZE but that's not enough concerning about one software for different hardware platforms there is a need to specify these sizes depending on the related platform and their ressources.

I was thinking about to use the base classes like ivector and ipool but it don't seem to be straight forward.

        const size_t SIZE = 512;
        etl::aligned_storage<sizeof(Data) * SIZE, etl::alignment_of<Data>::value>::type buffer;

        etl::ivector<Data> dyn_vector(buffer, SIZE); // 'etl::ivector<T>::ivector(T*, size_t) [with T = Data; size_t = long unsigned int]' is protected within this context

Is there another way to allocate dynamic memory at runtime and pass these memory buffers to ETL container?

jwellbelove commented 3 years ago

There are variants of many containers in the ETL that allow externally allocated memory or pools.

circular_buffer_ext
forward_list_ext
indirect_vector_ext
list_ext
string_ext
wstring_ext
u16_string_ext
U32_string_ext
vector_ext

Unfortunately deque does not currently have an '_ext' variant.

The size of the container is set by the external memory or pool. i.e.

int buffer[1000];
etl::vector_ext<int> vec(buffer, etl::size(buffer));
daniel-brosche commented 3 years ago

What is missing for my scenario is to create dynamically memory pools. Without dynamic pool creation (e.g. with external allocated memory buffer) there is, for example, a very limited possiblity to create a specified pool based on a configuration file (file contains the size of the pools). Depending on the hardware platform a pool could just contain 1000 or 100000 elements.

daniel-brosche commented 3 years ago

I have written some experimental code to realize pool_ext, generic_pool_ext and variant_pool_ext. https://github.com/daniel-brosche/etl/tree/feature/pool_ext

Did already some small tests with this versions and it seems to work. With some more work it could be good enough for a pull request?

jwellbelove commented 3 years ago

A pull request would be great when you are happy with them. Don't forget the unit tests!

daniel-brosche commented 3 years ago

I have several issues with the actual code base.

My feature branch is based on: a6d8a6d1 Updated version numbers John Wellbelove 27.09.2021 15:06:47

My compiler is: Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Debian 8.3.0-6' --with-bugurl=file:///usr/share/doc/gcc-8/README.Bugs --enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr --with-gcc-major-version-only --program-suffix=-8 --program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --enable-default-pie --with-system-zlib --with-target-system-zlib --enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-offload-targets=nvptx-none --without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 8.3.0 (Debian 8.3.0-6)

  1. There is a dependency to pthread (needed by stdlib) which needs to be addressesd in the meson build
    
    threads_dep = dependency('threads')

etl_unit_tests = executable('etl_unit_tests', include_directories: [ include_directories('test'),

Included here instead of with the dependency so we can see

    # header-induced warnings when building the tests
    include_directories('include')
],
sources: etl_test_sources,
dependencies: [unittestcpp_dep, threads_dep],
cpp_args: [
    '-fexceptions',
    '-DENABLE_ETL_UNIT_TESTS',
    '-DETL_DEBUG',
],
native: true,
install: false,
# Don't build tests by default if we are a subproject
build_by_default: meson.is_subproject() == false

)


2. some delegates tests failed (without any modifications)
```sh
../test/test_delegate.cpp:1021:1: error: Failure in test_call_if_and_not_valid_returning_void: !function_called
../test/test_delegate.cpp:1022:1: error: Failure in test_call_if_and_not_valid_returning_void: !parameter_correct
FAILURE: 1 out of 3934 tests failed (2 failures).

Besides that, is there something about code formating in the repository (e.g. clang-format file)?

jwellbelove commented 3 years ago

Are you able to debug step through the failing tests to see what the values of function_called and parameter_correct are being set to?

jwellbelove commented 3 years ago

There is currently no clang-format file. Code formatting should generally follow the precedent used in the rest of the library.

jwellbelove commented 3 years ago

pool_ext, generic_pool_ext and variant_pool_ext added in 20.18.0