On Tue, Aug 4, 2015, at 03:56 PM, Jeff Epler wrote: > On Tue, Aug 04, 2015 at 11:22:51AM -0500, Chris Lesiak wrote: > > I think there are problems in sampler_usr.c related to the use of fifo->out. > [snipped] > > Is the overwrite really required? Drop, as in streamer, is much simpler. > > This is the first serious look sampler/streamer have gotten in years, > I'm not surprised there's weird stuff to be found. > > No, I don't believe it's really required. In the unmerged branch where > I have factored out the common fifo code from sampler/streamer, I > dropped this overwrite feature and it does simplify the code in just the > way you say. (I need to rebase this branch onto 2.7 now that atomics > are in, so that I can use them there...) > > The fact that our testsuite passes after changing the overwrite code > means that at a minimum we weren't testing this feature. However, it > does seem that we *document* this behavior > sampler.N.curr-depth s32 output > Current number of samples in the FIFO. When this reaches > depth new data will begin overwriting old data, and some > samples will be lost. > > so I'll have to remember to drop that wording when I drop the feature.
I would tend to argue that overwrite is the proper behavior. Realtime is spitting out data. User-space can call behind and the FIFO can fill up. The original functionality was intended to "fail gently" in this situation by overwriting the oldest data instead of dropping the new stuff. I suppose you could argue that if the FIFO fills up you're screwed regardless, but I still think eliminating that functionality is a step backwards. That said, I'm not in a position to do the coding to make it work right myself, so I guess I shouldn't complain. > Thanks so much for taking the time to read through this code. It's > something we will benefit from doing more and more. (There are more > code reviews going on, but mostly on our IRC channel) > > Jeff > -- John Kasunich [email protected] ------------------------------------------------------------------------------ _______________________________________________ Emc-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/emc-developers
