Beautify T4 of mpc.exe and reduce local variables of the deserialization by pCYSl5EDgo · Pull Request #1074 · MessagePack-CSharp/MessagePack-CSharp

pCYSl5EDgo · 2020-09-30T15:33:49Z

Refactor common codes between int/string key into function.
More strict namespace.

Add some logic to the string-key.
Optimize int-key when member array is empty.

I tried reducing the using System.Buffers; and using MessagePack; but I failed.

…into master

Refactor common codes between int/string key into function More strict namespace Add some logic to the string-key Optimize int-key when member array is empty

pCYSl5EDgo · 2020-10-01T11:11:26Z

Added some optimizations.

During deserialization

If there is no members, just Skip().
If the constructor has no parameters, writes directly to the members.
- This reduces unnecessary local variables and assignments.

sandbox/TestData2/Generated.cs

pCYSl5EDgo · 2020-10-14T06:32:54Z

I think about that braking change.
It only affects string-key types which has a parameter-less constructor and not default-initialized serializable and writable field/properties when the encoded byte sequence does not include those fields' string-key.
The impact range is limited.

If the object can be created before acutual deserialization, the stack local variables can be removed.

pCYSl5EDgo · 2020-10-17T06:27:54Z

Circle CI is unstable.
The errors are thrown during the environment setup process.

AArnott · 2020-10-17T18:31:15Z

Circle CI is unstable.

We're not using Circle CI.

Current dynamic IL generating resolver emits the codes which do set members to their default values...
I have another PR to change dynamic
resolver's behaviour to reduce stack allocation.

Why is the stack allocation important?
We want to keep the AOT and dynamic resolver behavior equivalent. So the two changes should be merged together first and then submitted as one coherent PR so that AOT and dynamic resolver change together.

I think about that braking change.
It only affects string-key types which has a parameter-less constructor and not default-initialized serializable and writable field/properties when the encoded byte sequence does not include those fields' string-key.
The impact range is limited.

It's unclear to me what the value is though. We should only take breaking changes if their value significantly outweighs the risk of breaking folks, and I don't see reducing the stack size as significantly valuable. Are you aware of something that makes this valuable?

AArnott

Rejecting this till the breaking change is justified and the dynamic resolver change can be made at the same time, with tests.

This behavior may not be desirable, but the test documents current behavior. See MessagePack-CSharp#1074 (comment)

pCYSl5EDgo · 2020-10-18T14:24:06Z

@AArnott
I do not find the test C# file which examines the formatters generated by mpc.exe.
Where is it?
Should I create new csproject to test those formatters generated by mpc.exe?

I made a DynamicResolver change and test for it.
The undesirable behavior is fixed in all the 2^3 ==8 cases(Dynamic and mpc.exe, int-key and string-key, and class and struct).

It's unclear to me what the value is though. We should only take breaking changes if their value significantly outweighs the risk of breaking folks, and I don't see reducing the stack size as significantly valuable. Are you aware of something that makes this valuable?

It is valuable.
SkipLocalsInitAttribute will appear since .NET 5. .NET's PR
C# initializes local variables with the default value.
The programmers have paid the initialization cost twice.
IL2CPP zero-ing is inefficient. It initializes each of the local variables and a lot of function calls happen.

If the programmers can avoid the local variables, the programmers should. (It is a good act to write local variables to help them understand as far as C# compiler can remove the local variables)
This parameter-less constructor case is the best suitable case.

This is a refactoring of int-key mpc.exe tt file.

* This is a cherry-pick of #1074. This is a refactoring of int-key mpc.exe tt file. * Change the format to decrease template part lines. Change the iterator `i` type from `int` to `var`. * fix CRLF * Change: var to globall:: Revert: DepthStep position Revert: }#> -> } #> * Fix: space * Fix: Line Feed * Fix: var -> global::MessagePack.IFormatterResolver

…rp#1092 commit d7a50b9 Merge: 02ae418 5a6cda6 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Mon Oct 26 09:33:45 2020 +0900 Merge remote-tracking branch 'upstream/develop' into solution#1085-mpc commit 02ae418 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Mon Oct 26 09:25:20 2020 +0900 Update: Loop demonstration commit d84ae75 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Mon Oct 26 09:24:31 2020 +0900 Change: drop deserialized value -> reader.Skip() commit 9b0d4a2 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Mon Oct 26 00:27:10 2020 +0900 Fix: remove unused boolean local variables commit fdfabeb Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Mon Oct 26 00:00:28 2020 +0900 Update: Make the same as Dynamic commit bef4692 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Sun Oct 25 19:37:11 2020 +0900 Add test project commit a8fac80 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Sun Oct 25 18:55:27 2020 +0900 Add types to sandbox commit 6ffc3c3 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Sun Oct 25 18:45:40 2020 +0900 Update: string-key mpc.exe commit 44819e6 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Sun Oct 25 18:05:24 2020 +0900 Update: int-key mpc.exe commit 0a2fef4 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Sun Oct 25 12:12:57 2020 +0900 Fix: var -> global::MessagePack.IFormatterResolver commit 086aa11 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Sun Oct 25 12:05:21 2020 +0900 Fix: Line Feed commit 39d3d08 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Sun Oct 25 12:03:37 2020 +0900 Fix: space commit c57f423 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Sun Oct 25 12:00:32 2020 +0900 Change: var to globall:: Revert: DepthStep position Revert: }#> -> } #> commit de6fd23 Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Sat Oct 24 11:04:44 2020 +0900 fix CRLF commit 4b6ae7c Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Sat Oct 24 10:59:50 2020 +0900 Change the format to decrease template part lines. Change the iterator `i` type from `int` to `var`. commit 95f41ad Author: pCYSl5EDgo <pCYSl5EDgo@yahoo.co.jp> Date: Fri Oct 23 21:03:54 2020 +0900 This is a cherry-pick of MessagePack-CSharp#1074. This is a refactoring of int-key mpc.exe tt file.

This behavior may not be desirable, but the test documents current behavior. See MessagePack-CSharp/MessagePack-CSharp#1074 (comment)

pCYSl5EDgo added 6 commits September 24, 2020 22:45

Merge remote-tracking branch 'upstream/master' into master

bf7f39d

Merge branch 'master' of https://github.com/neuecc/MessagePack-CSharp …

d6806f5

…into master

Beautify format

e8f0c65

Refactor common codes between int/string key into function More strict namespace Add some logic to the string-key Optimize int-key when member array is empty

Remove unnecessary local variables

2214dd3

Fix: overwrite only writable members

0406b1b

Fix: if must be nested

5f7e9c4

AArnott reviewed Oct 8, 2020

View reviewed changes

sandbox/TestData2/Generated.cs Show resolved Hide resolved

AArnott approved these changes Oct 8, 2020

View reviewed changes

pCYSl5EDgo added 2 commits October 17, 2020 14:42

Make Dynamic IL Generator behaviour the same

f311db9

If the object can be created before acutual deserialization, the stack local variables can be removed.

Rerun CI pipeline

081445b

pCYSl5EDgo changed the title ~~Beautify T4 format of int-key formatter~~ Beautify T4 of mpc.exe and reduce local variables of the deserialization Oct 17, 2020

AArnott requested changes Oct 17, 2020

View reviewed changes

AArnott closed this Oct 17, 2020

AArnott added a commit to AArnott/MessagePack-CSharp that referenced this pull request Oct 17, 2020

Verify that missing properties are initialized to their default value

1c1c2ea

This behavior may not be desirable, but the test documents current behavior. See MessagePack-CSharp#1074 (comment)

AArnott added a commit to AArnott/MessagePack-CSharp that referenced this pull request Oct 18, 2020

Verify that missing properties are initialized to their default value

2f246d2

This behavior may not be desirable, but the test documents current behavior. See MessagePack-CSharp#1074 (comment)

pCYSl5EDgo added a commit to pCYSl5EDgo/MessagePack-CSharp that referenced this pull request Oct 23, 2020

This cherry-pick of MessagePack-CSharp#1074.

5314e5d

This is a refactoring of int-key mpc.exe tt file.

pCYSl5EDgo added a commit to pCYSl5EDgo/MessagePack-CSharp that referenced this pull request Oct 23, 2020

This is a cherry-pick of MessagePack-CSharp#1074.

95f41ad

This is a refactoring of int-key mpc.exe tt file.

This was referenced Oct 23, 2020

Refactoring of int-key mpc.exe tt file #1088

Merged

Support where constraints in mpc.exe string-key. #1089

Merged

AArnott mentioned this pull request Oct 24, 2020

Use contractless resolver without setting default values for missing properties #1085

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Beautify T4 of mpc.exe and reduce local variables of the deserialization#1074

Beautify T4 of mpc.exe and reduce local variables of the deserialization#1074
pCYSl5EDgo wants to merge 8 commits intoMessagePack-CSharp:masterfrom
pCYSl5EDgo:mpc-fix

pCYSl5EDgo commented Sep 30, 2020

Uh oh!

pCYSl5EDgo commented Oct 1, 2020 •

edited

Loading

Uh oh!

Uh oh!

pCYSl5EDgo commented Oct 14, 2020

Uh oh!

pCYSl5EDgo commented Oct 17, 2020

Uh oh!

AArnott commented Oct 17, 2020

Uh oh!

AArnott left a comment

Uh oh!

pCYSl5EDgo commented Oct 18, 2020 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pCYSl5EDgo commented Sep 30, 2020

Uh oh!

pCYSl5EDgo commented Oct 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

pCYSl5EDgo commented Oct 14, 2020

Uh oh!

pCYSl5EDgo commented Oct 17, 2020

Uh oh!

AArnott commented Oct 17, 2020

Uh oh!

AArnott left a comment

Choose a reason for hiding this comment

Uh oh!

pCYSl5EDgo commented Oct 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pCYSl5EDgo commented Oct 1, 2020 •

edited

Loading

pCYSl5EDgo commented Oct 18, 2020 •

edited

Loading