Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get_stream_buffer_length() #12

Merged
merged 6 commits into from
Jun 6, 2019
Merged

get_stream_buffer_length() #12

merged 6 commits into from
Jun 6, 2019

Conversation

DaWe35
Copy link
Contributor

@DaWe35 DaWe35 commented Jun 6, 2019

I needed a function that returns the number of items in buffer. If you find it useful, feel free to pull :)
(And the performance of get_stream_buffer_length() is better than len(self.stream_buffer), but I don't think it matters very much.)

@oliver-zehentleitner
Copy link
Member

I see the point of the improvment! Thank you very much!

The default value of pop() is -1, the last (youngest) elem of the array, not the oldest.
@DaWe35
Copy link
Contributor Author

DaWe35 commented Jun 6, 2019

#5 commit: pop() changed to pop(0), I think it was a bug

I read the thing afterwards, len() does the same think what I did
@oliver-zehentleitner oliver-zehentleitner self-assigned this Jun 6, 2019
@oliver-zehentleitner oliver-zehentleitner added the enhancement New feature or request label Jun 6, 2019
@oliver-zehentleitner
Copy link
Member

oliver-zehentleitner commented Jun 6, 2019

I think your first suggestion is correct! len() is counting all elements of the list every time its used. If we the use append and pop methods to increase or decrease with 1, then for sure its faster to use self.stream_buffer_length instead of len(array).

if someone has the reason to control every second the stream_buffer size, then your first solution is definitely the best way ...

#5 commit: pop() changed to pop(0), I think it was a bug

Yes, it should be a FIFO stack, thanks!

@DaWe35
Copy link
Contributor Author

DaWe35 commented Jun 6, 2019

Hmm. I don't know. It would be logical, but I think python already does that. There is a conversion about len(): https://stackoverflow.com/questions/699177/python-do-python-lists-keep-a-count-for-len-or-does-it-count-for-each-call
If you decide to use self.stream_buffer_length instead of len(array), just ignore #6 (a16e9aa)commit.

@oliver-zehentleitner
Copy link
Member

oliver-zehentleitner commented Jun 6, 2019

i am thinking about it ... on the other hand, the stream_buffer should be almost 0 all the time, so its a bigger waste to count hundreds of +/- 1 operations per second, if you dont need them.

@oliver-zehentleitner oliver-zehentleitner merged commit 8b86044 into LUCIT-Systems-and-Development:master Jun 6, 2019
@DaWe35
Copy link
Contributor Author

DaWe35 commented Jun 6, 2019

True. And what is with sys.getsizeof? :)

@DaWe35
Copy link
Contributor Author

DaWe35 commented Jun 6, 2019

I can't update the documentation (https://www.unicorn-data.com/unicorn-binance-websocket-api.html), so if you want, you need to add get_stream_buffer_length()

@oliver-zehentleitner
Copy link
Member

oliver-zehentleitner commented Jun 6, 2019

True. And what is with sys.getsizeof? :)

Its because, if I count the size this way, its compare able to the self.total_received_bytes value. But if i use the size of the stream_buffer variable itself, its very different. not only the raw data is used in the size, also the metadata for the complete list object of the stream_buffer and we receive a json and convert it! But at least its not accurate information, we should switch to use the data from the stream_buffer itself.

I can't update the documentation (https://www.unicorn-data.com/unicorn-binance-websocket-api.html), so if you want, you need to add get_stream_buffer_length()

Thanks, I will do it asap!

@oliver-zehentleitner
Copy link
Member

@DaWe35
Copy link
Contributor Author

DaWe35 commented Jun 6, 2019

Cooool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants