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

Add .NET debugging attributes #14097

Closed
wants to merge 1 commit into from

Conversation

JamesNK
Copy link
Contributor

@JamesNK JamesNK commented Sep 15, 2023

I've been working with a large Protobuf model and noticed debugging issues that can easily be improved.

Add debugging attributes to collections, ByteString, and descriptors. No impact on runtime behavior. Debugger type proxies are what the debugger displays by default, but full data is available by selecting "Raw View".

RepeatedField and MapField now display their item count. This is standard across .NET collections. For example, .NET's list and dictionary both display Count = {Count}.

Note that previously, Protobuf collections displayed the result of ToString, which returned JSON. The JSON debug view isn't useful when there is too much content to display at once because it's truncated. That experience will be fairly common.

Count benefits:

  • Always useful
  • Makes debugging Protobuf collections feel more like regular .NET collections
  • Collection contents is now easier to access with debugger type proxies

No dependency between this PR and #13838. Each can be merged independently.

MapField before:

image

MapField after:

image

RepeatedField before:

image

RepeatedField after:

image

ByteString before:

image

ByteString after:

image

Descriptor before:

image

Descriptor after:

image

TypeRegistry before:

image

TypeRegistry after:

image

@JamesNK JamesNK requested a review from a team as a code owner September 15, 2023 14:43
@JamesNK JamesNK requested review from jskeet and removed request for a team September 15, 2023 14:43
@JamesNK
Copy link
Contributor Author

JamesNK commented Sep 15, 2023

cc @jtattermusch

@jskeet
Copy link
Contributor

jskeet commented Sep 18, 2023

Will look at this tomorrow - I've got Covid so I'm working more slowly in general. Will get to it eventually...

@@ -23,6 +24,8 @@ namespace Google.Protobuf
/// Immutable array of bytes.
/// </summary>
[SecuritySafeCritical]
[DebuggerDisplay("Length = {Length}")]
Copy link
Contributor Author

@JamesNK JamesNK Sep 18, 2023

Choose a reason for hiding this comment

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

Note: ByteString initially displays length rather than count to act more like array/span/memory. Also, ByteString has a Length property rather than a Count property.

Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Love it! I particularly like the fact that there's no need to change the generated code at all :)

@jskeet jskeet added the c# label Sep 19, 2023
@jskeet jskeet self-assigned this Sep 19, 2023
@jskeet jskeet added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Sep 19, 2023
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Sep 19, 2023
@jskeet jskeet requested review from mkruskal-google and removed request for deannagarcia September 20, 2023 07:54
copybara-service bot pushed a commit that referenced this pull request Sep 22, 2023
I've been working with a large Protobuf model and noticed debugging issues that can easily be improved.

Add debugging attributes to collections, `ByteString`, and descriptors. No impact on runtime behavior. Debugger type proxies are what the debugger displays by default, but full data is available by selecting "Raw View".

`RepeatedField` and `MapField` now display their item count. This is standard across .NET collections. For example, .NET's list and dictionary both display `Count = {Count}`.

Note that previously, Protobuf collections displayed the result of `ToString`, which returned JSON. The JSON debug view isn't useful when there is too much content to display at once because it's truncated. That experience will be fairly common.

Count benefits:
* Always useful
* Makes debugging Protobuf collections feel more like regular .NET collections
* Collection contents is now easier to access with debugger type proxies

No dependency between this PR and #13838. Each can be merged independently.

**MapField before:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/9dd3baa9-4432-446e-9049-1f7268d5be4c)

**MapField after:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/ac4aea33-e339-49e0-9a67-c174d2608393)

**RepeatedField before:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/16353785-bef4-4489-a3ab-de2437d51d4e)

**RepeatedField after:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/f3de7680-ded9-41d8-aac0-84a9e7b65c98)

**ByteString before:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/4febc400-1eb7-46ee-911e-a7698783a358)

**ByteString after:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/7635080e-9bb0-4f61-9a39-afbb9e575051)

**Descriptor before:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/5b89792b-a16a-4641-a50c-5355b5230b5d)

**Descriptor after:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/5047dc1e-c93b-43d5-bb8c-a6976a9ae6da)

**TypeRegistry before:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/d4aedf19-22cc-49b5-8717-9299e00abc85)

**TypeRegistry after:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/4a473595-74b9-40e2-96b8-2b103d5f44b0)

Closes #14097

COPYBARA_INTEGRATE_REVIEW=#14097 from JamesNK:jamesnk/debugging f0dea34
FUTURE_COPYBARA_INTEGRATE_REVIEW=#14097 from JamesNK:jamesnk/debugging f0dea34
PiperOrigin-RevId: 567636087
copybara-service bot pushed a commit that referenced this pull request Sep 26, 2023
I've been working with a large Protobuf model and noticed debugging issues that can easily be improved.

Add debugging attributes to collections, `ByteString`, and descriptors. No impact on runtime behavior. Debugger type proxies are what the debugger displays by default, but full data is available by selecting "Raw View".

`RepeatedField` and `MapField` now display their item count. This is standard across .NET collections. For example, .NET's list and dictionary both display `Count = {Count}`.

Note that previously, Protobuf collections displayed the result of `ToString`, which returned JSON. The JSON debug view isn't useful when there is too much content to display at once because it's truncated. That experience will be fairly common.

Count benefits:
* Always useful
* Makes debugging Protobuf collections feel more like regular .NET collections
* Collection contents is now easier to access with debugger type proxies

No dependency between this PR and #13838. Each can be merged independently.

**MapField before:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/9dd3baa9-4432-446e-9049-1f7268d5be4c)

**MapField after:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/ac4aea33-e339-49e0-9a67-c174d2608393)

**RepeatedField before:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/16353785-bef4-4489-a3ab-de2437d51d4e)

**RepeatedField after:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/f3de7680-ded9-41d8-aac0-84a9e7b65c98)

**ByteString before:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/4febc400-1eb7-46ee-911e-a7698783a358)

**ByteString after:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/7635080e-9bb0-4f61-9a39-afbb9e575051)

**Descriptor before:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/5b89792b-a16a-4641-a50c-5355b5230b5d)

**Descriptor after:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/5047dc1e-c93b-43d5-bb8c-a6976a9ae6da)

**TypeRegistry before:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/d4aedf19-22cc-49b5-8717-9299e00abc85)

**TypeRegistry after:**

![image](https://github.com/protocolbuffers/protobuf/assets/303201/4a473595-74b9-40e2-96b8-2b103d5f44b0)

Closes #14097

COPYBARA_INTEGRATE_REVIEW=#14097 from JamesNK:jamesnk/debugging f0dea34
FUTURE_COPYBARA_INTEGRATE_REVIEW=#14097 from JamesNK:jamesnk/debugging f0dea34
PiperOrigin-RevId: 567636087
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants