Strip DataServiceInterface and rewrite DataService#40
Conversation
…owing data overlaps by default
…into feature/data_service_rewrite
whyitfor
left a comment
whyitfor
left a comment
There was a problem hiding this comment.
This seems to be coming along well. It is much more concise!
See some comments. I think the docstrings you are planning on adding will definitely help with readability.
…lized and deserialize data service
EdwardLarson
changed the title
whyitfor
left a comment
whyitfor
left a comment
There was a problem hiding this comment.
Looks good. I left a few comments.
|
By the way, I did the profiling that we discussed offline, and for a recursive_unpack on busybox that takes 15 min, the new data service was faster overall by a few seconds, which isn't that significant. But what's notable is that abut 16s less was spent in the new data service compared to the old one, out of abut 30s spent in the data service total (originally). So I don't know if I'd call it a 50% performance improvement, but it's certainly not a drop, and it isn't going to make the data service the bottleneck. |
whyitfor
left a comment
whyitfor
left a comment
There was a problem hiding this comment.
Looks good! Only outstanding thing seems to be updating ofrak test coverage figure from 85 to 84
- Bump mkdocs to 1.2.3, as 1.2.2 is exposed to GHSA-qh9q-34h6-hcv9 - Update broken links in NAND Flash Components documentation - Update Resource docstrings which reference parameters removed in redballoonsecurity#40 - Update ResourceServiceInterface docstrings which reference parameters deprecated in redballoonsecurity#40
- Bump mkdocs to 1.2.3, as 1.2.2 is exposed to GHSA-qh9q-34h6-hcv9 - Update broken links in NAND Flash Components documentation - Update Resource docstrings which reference parameters removed in #40 - Update ResourceServiceInterface docstrings which reference parameters deprecated in #40
…tGeneration Feature/script generation
3 participants
Link to Related Issue(s)
Please describe the changes in your request.
Strips a number of unused features and methods from the DataService.
Includes a new DataService implementation which is simpler and should be easier to maintain.
Anyone you think should look at this, specifically?
@whyitfor, @rbs-jacob, and @andresito00 , do you agree that this new version is easier to understand? Especially look at the
_apply_patches_to_rootmethod and the_DataRoot+_Waypointmodel of data mapping. Are these easy to understand?