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

remove the boolean array in global_queue, as only non-zero values are pu... #68

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@puqing
Contributor

puqing commented Feb 24, 2014

...shed in

There is a `flag' array in the global_queue structure. It is 64k bytes long. Since the message_queue pointers pushed in global_queue are all non-zero, this array is unnecessary, because we can directly check the value of the pointers.

@cloudwu

This comment has been minimized.

Show comment
Hide comment
@cloudwu

cloudwu Feb 24, 2014

Owner

Flags in global queue is for concurrent, because change a 64 bits pointer may not be atomic. so don't remove them.

Owner

cloudwu commented Feb 24, 2014

Flags in global queue is for concurrent, because change a 64 bits pointer may not be atomic. so don't remove them.

@cloudwu cloudwu closed this Feb 24, 2014

@puqing puqing deleted the puqing:new-branch branch Feb 24, 2014

@cloudwu

This comment has been minimized.

Show comment
Hide comment
@cloudwu

cloudwu May 7, 2014

Owner

Pointer on a 64bit OS on a 64bit CPU is atomicity, so I will remove flags array, thank you for the pr :)

http://stackoverflow.com/questions/3123326/atomicity-in-32-64-bit

Owner

cloudwu commented May 7, 2014

Pointer on a 64bit OS on a 64bit CPU is atomicity, so I will remove flags array, thank you for the pr :)

http://stackoverflow.com/questions/3123326/atomicity-in-32-64-bit

@cloudwu

This comment has been minimized.

Show comment
Hide comment
@cloudwu

cloudwu May 7, 2014

Owner

I remember why we use a flags array separately. The reason is not 64it atomic.

Because when we push a mq to global queue, we should push the pointer first, and then set the flag. only then the pop can ensure the pointer is the new version.

Owner

cloudwu commented May 7, 2014

I remember why we use a flags array separately. The reason is not 64it atomic.

Because when we push a mq to global queue, we should push the pointer first, and then set the flag. only then the pop can ensure the pointer is the new version.

@cloudwu

This comment has been minimized.

Show comment
Hide comment

@puqing puqing restored the puqing:new-branch branch May 9, 2014

@puqing puqing deleted the puqing:new-branch branch May 9, 2014

@puqing puqing restored the puqing:new-branch branch May 9, 2014

@puqing

This comment has been minimized.

Show comment
Hide comment
@puqing

puqing May 9, 2014

Contributor

Hi, sorry for the late reply. It's good to know the 64-bit pointer operations are atomic. I was not very aware of this issue before you mentioned it, and I think it's a very interesting topic to consider in the code.

Although the stackoverflow page has mentioned that pointer operations are atomic, one thing I'm still not very sure is whether the values in the queue will be treated as "integers" or "pointers", as the operations here are all assignments and comparisons, but not dereferencing.

I "restored" the branch so that you could take a look at the code. Maybe it is still of a little use.

Contributor

puqing commented May 9, 2014

Hi, sorry for the late reply. It's good to know the 64-bit pointer operations are atomic. I was not very aware of this issue before you mentioned it, and I think it's a very interesting topic to consider in the code.

Although the stackoverflow page has mentioned that pointer operations are atomic, one thing I'm still not very sure is whether the values in the queue will be treated as "integers" or "pointers", as the operations here are all assignments and comparisons, but not dereferencing.

I "restored" the branch so that you could take a look at the code. Maybe it is still of a little use.

@puqing

This comment has been minimized.

Show comment
Hide comment
@puqing

puqing May 9, 2014

Contributor

Btw, in the blog, you mentioned an issue about the "context handle". You have addressed the issue long ago. But since the context object and its corresponding "message_queue" are always related 1-to-1, maybe they could be bundled together, so that they are created and destroyed always at the same time. That is, put the message_queue structure, instead of its pointer, inside the skynet_context structure, and use the "." operator and the "offsetof" macro to reference each other.

Since the current code works just fine, maybe this is an unneccesary modification, but I think it could make the code more robust. Just my two cents.

Contributor

puqing commented May 9, 2014

Btw, in the blog, you mentioned an issue about the "context handle". You have addressed the issue long ago. But since the context object and its corresponding "message_queue" are always related 1-to-1, maybe they could be bundled together, so that they are created and destroyed always at the same time. That is, put the message_queue structure, instead of its pointer, inside the skynet_context structure, and use the "." operator and the "offsetof" macro to reference each other.

Since the current code works just fine, maybe this is an unneccesary modification, but I think it could make the code more robust. Just my two cents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment