Skip to content

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

Closed
puqing wants to merge 1 commit intocloudwu:masterfrom
puqing:new-branch
Closed

remove the boolean array in global_queue, as only non-zero values are pu...#68
puqing wants to merge 1 commit intocloudwu:masterfrom
puqing:new-branch

Conversation

@puqing
Copy link
Copy Markdown
Contributor

@puqing 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
Copy link
Copy Markdown
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 new-branch branch February 24, 2014 05:45
@cloudwu
Copy link
Copy Markdown
Owner

cloudwu commented May 7, 2014

@cloudwu
Copy link
Copy Markdown
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.

@puqing puqing restored the new-branch branch May 9, 2014 01:12
@puqing puqing deleted the new-branch branch May 9, 2014 01:15
@puqing puqing restored the new-branch branch May 9, 2014 01:16
@puqing
Copy link
Copy Markdown
Contributor Author

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
Copy link
Copy Markdown
Contributor Author

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants