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

gfxrecon-replay - handle wrap around of xcb events sequence numbers #1261

Merged

Conversation

davidlunarg
Copy link
Contributor

The sequence number in the cookie returned by many xcb calls (i.e. xcb_map_window) is 32 bits. The sequence number in the event returned by xcb_wait_for_event is 16 bits. When comparing the two sequence numbers to see if the xcb call has completed, need to mask off the top 16 bits from the cookie sequence number.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 45799.

@davidlunarg
Copy link
Contributor Author

Still working on this change, but thought I would a PR to see how it does on CI.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3217 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3217 passed.

@davidlunarg davidlunarg changed the title gfxrecon-replay - handle wrapparound of xcb events sequence numbers gfxrecon-replay - handle wrap around of xcb events sequence numbers Sep 20, 2023
The sequence number in the cookie returned by many xcb calls
(i.e. xcb_map_window) is 32 bits.  The sequence number in the event
returned by xcb_wait_for_event is 16 bits. When comparing the two
sequence numbers to see if the xcb call has completed, mask off the
top 16 bits from the cookie sequence number before doing the compare.
@davidlunarg davidlunarg force-pushed the davidp_xcb_event_sequence_wraparound2 branch from e953303 to 7c6857a Compare September 20, 2023 17:47
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 46436.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3227 running.

@davidlunarg
Copy link
Contributor Author

This is no longer a draft PR. I would like to have it reviewed so I can merge it.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 3227 passed.

Copy link
Contributor

@MarkY-LunarG MarkY-LunarG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Good comment as well.

Copy link
Contributor

@andrew-lunarg andrew-lunarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Alternative 1: We don't really need to change the function signatures, just the comparison (but then we'd need to mask both side of the comparison for safety).

Alternative 2: We could make EventInfo.sequence uint16_t, which would do the throwing away of bits upon assignment to it and thereby avoid the need to mask.

Took me a moment digging in to xcb docs to grok the comment. Also, arg, why is sequence 32 bit here in xcb and 16 bit there in it???

@davidlunarg
Copy link
Contributor Author

Looks good to me.

Alternative 1: We don't really need to change the function signatures, just the comparison (but then we'd need to mask both side of the comparison for safety).

Alternative 2: We could make EventInfo.sequence uint16_t, which would do the throwing away of bits upon assignment to it and thereby avoid the need to mask.

Took me a moment digging in to xcb docs to grok the comment. Also, arg, why is sequence 32 bit here in xcb and 16 bit there in it???

I changed the function signatures to accurately reflect what is being passed - the sequence number is 16 bits as it arrived from xcb, so we won't mislead someone reading the code that it is 32 bits.

Why does xcb use 32 bits in the sequence number in the cookie that is returned by xcb calls, but 16 bits in the notification event? I started out thinking that 16 bits is used in the notification event in an attempt to save memory, since lots of events are generated. But the events are problably short-lived and the memory re-used, so the amount of extra memory used would be minimal. I dunno.

@andrew-lunarg
Copy link
Contributor

I changed the function signatures to accurately reflect what is being passed - the sequence number is 16 bits as it arrived from xcb, so we won't mislead someone reading the code that it is 32 bits.

Why does xcb use 32 bits in the sequence number in the cookie that is returned by xcb calls, but 16 bits in the notification event? I started out thinking that 16 bits is used in the notification event in an attempt to save memory, since lots of events are generated. But the events are problably short-lived and the memory re-used, so the amount of extra memory used would be minimal. I dunno.

Makes sense!

I asked a friend and this is what they said:

Why is the sequence number sometimes 32 bit and sometimes 16 bit in the XCB API?

The sequence number in the XCB API can be either 32-bit or 16-bit depending on the context. The sequence number is used to uniquely identify each request or event sent between the X client and the X server.

In the XCB API, the sequence number is represented by the xcb_void_cookie_t type. This type is defined as a 32-bit value when the sequence number needs to be 32 bits, and as a 16-bit value when the sequence number can be represented by 16 bits.

The reason for having both 32-bit and 16-bit sequence numbers is to optimize memory usage and network bandwidth. In situations where the number of requests or events is expected to be small, a 16-bit sequence number is sufficient and reduces the overhead of larger data types. On the other hand, in scenarios where a large number of requests or events are expected, a 32-bit sequence number is used to accommodate the larger range of values.

Here are some additional details regarding the sequence numbers in the XCB API:

  • In the XCB protocol specification, the sequence number is defined as a CARD16 (16-bit) value by default. This is the most common case and is used when the number of requests or events is expected to be within the range of a 16-bit value.
  • In situations where the number of requests or events exceeds the range of a 16-bit value, the XCB protocol allows for the use of a 32-bit sequence number. This is achieved by extending the sequence number field to a CARD32 (32-bit) value, which provides a much larger range for sequence numbers.
  • The decision to use a 32-bit or 16-bit sequence number is typically made by the X server implementation. It depends on factors such as the expected number of simultaneous clients, the expected rate of requests and events, and the available resources.
  • When using the XCB API, developers don't need to manually handle the sequence numbers. The XCB library takes care of managing the sequence numbers internally and ensures that each request or event is assigned a unique sequence number.

In summary, the sequence number in the XCB API can be either 32-bit or 16-bit depending on the situation. The choice of using a 32-bit or 16-bit sequence number is determined by factors such as the expected number of requests or events and the available resources. The XCB library handles the management of sequence numbers internally, so developers don't need to worry about it in most cases.

https://www.phind.com/search?cache=h3ji83h983v3rgh0agfkmnbp

@davidlunarg davidlunarg merged commit f8046bc into LunarG:dev Sep 21, 2023
@davidlunarg davidlunarg deleted the davidp_xcb_event_sequence_wraparound2 branch September 21, 2023 16:42
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.

5 participants