device_tree-rs icon indicating copy to clipboard operation
device_tree-rs copied to clipboard

Best way to handle fork optimized for sparse reading

Open posborne opened this issue 8 years ago • 1 comments

CC @nastevens

Hello @mbr. Thanks for putting together this library -- it was great for giving me something to work with starting out for a problem I have been working on.

Unfortunately, the design of the current parser (which requires that the entire device tree be loaded into a buffer in RAM and then a second copy be stored in the data structures) was not suitable for my use cases (In my case, I am dealing with FIT images that contain full kernels and ramdisks weighing in at ~20MB+).

For my use case, I only really care about one specific piece of information hidden in the device tree so I ended up writing a new device tree parser (based partially on the API and implementation of this library) that creates a much lighter weight data structure by reading/seeking out of a file rather than out of a memory buffer. This way, I do not have to pay the memory/cpu cost of allocating (or even reading) large property values from the input file.

I think what I have put together could be useful for other users, and I could see a couple ways of dealing with the change:

  1. Open a PR here that will replace the current library with the new implementation
  2. Offer up the code as a separate repo/crate (also MIT licensed and including your Copyright as well as mine).

Although the new implementation was much more optimized for my current use cases, I can see a few drawbacks to using it:

  • My implementation depends on std as well as the byteorder crate.
  • The original implementation is probably faster for small device trees where all of the data will be used (and especially if the property keys/values will be accessed many times).
  • At the moment, mine lacks a few of the features related to parsing property values.

Thoughts on how to move forward? Right now I am leaning toward the second option.

posborne avatar Jun 14 '16 01:06 posborne

Hey @posborne, @nastevens

Unfortunately, the design of the current parser (which requires that the entire device tree be loaded into a buffer in RAM and then a second copy be stored in the data structures) was not suitable for my use cases (In my case, I am dealing with FIT images that contain full kernels and ramdisks weighing in at ~20MB+).

that is almost a bit tragic - the lib came out of some current thesis work and it has seen two rewrites; my initial version did not store anything in memory, but instead read it everything from the device tree on the flash/file.

My usecase is code-generation though, on a "real" system. The Format is somewhat suitable for traversal and search, but not completely - I had a roughly working version but it turned out to be very painful to use if you're doing what I am doing right now. So that approach was ditched in favor of slightly less efficient code that was much simpler to write, use and maintain. This, of course, dropped the use-case of "minimal resources"; I figured a few bytes of RAM to load a device tree were okay even in other embedded projects. It seems I was wrong.

I think what I have put together could be useful for other users, and I could see a couple ways of dealing with the change:

Open a PR here that will replace the current library with the new implementation Offer up the code as a separate repo/crate (also MIT licensed and including your Copyright as well as mine).

I would be nice to support both use-cases, maybe it would be possible to have a smaller traversal API that allows building the parse tree, if you so desire?

Although the new implementation was much more optimized for my current use cases, I can see a few drawbacks to using it:

My implementation depends on std as well as the byteorder crate.

The nostd is in there because I though one would want to run this on bare-metal at some point.

Thoughts on how to move forward? Right now I am leaning toward the second option.

If it is okay, I'll email you directly for a speedier discussion.

Thanks for the input!

mbr avatar Jun 15 '16 11:06 mbr