Conversation
|
Looks good! Just the typo I pointed and drop the linter's changes to |
d08e039 to
821e4ec
Compare
ea1bdc9 to
49bbe26
Compare
49bbe26 to
c0fe7b1
Compare
c0fe7b1 to
aae0ca3
Compare
aae0ca3 to
10b7ae9
Compare
|
looks fine to me. a note for future reference, if you could refrain from having a significant chunk of your changes be formatting changes only that'd be much appreciated. I'm all for have formatting changes to make the formatting more consistent but having them mixed with actual functional changes makes CRs more work than it should be. I'm not gonna ask you to separate them out for this PR but please keep it in mind for future PRs. I'm guessing these changes simply come from whatever editor you are using that does the formatting. you can just do that once and submit a PR to cover all formatting changes. I see this is marked as Draft, please let me know when you think this is ready for checkin. |
f8f5568 to
c2170e0
Compare
|
Once the formatting change PR is merged, changes to GCPerfSim.cs should be purely functional changes. |
db3eb48 to
7d6ea44
Compare
7bc2dd2 to
ca3f457
Compare
96e15c3 to
03890f4
Compare
03890f4 to
6132643
Compare
c2bb137 to
965d036
Compare
|
This PR is subsumed by #1704 |
In order to debug what is wrong with the POH allocation test case, I added this support in
GCPerfSimto count how many bytes we allocated into the individual object heaps.To start with the review, read the changes in
GCPerfSim.cs.The change accounts for the bytes allocated in individual object heaps in two ways. First, it is done in
GetObjectSpec()and then it is done again in theStatisticsclass which is enabled only for debug, which allows me to be sure that the count is correct when I debug.The change I did found a bug. In order for this accounting scheme to work, we need to make sure whenever
GetObjectSpec()is called, there is exactly one allocation according to the spec happen downstream inJustMakeObject(). Just so it is clear, we need this constraint for another reason, if that is not true, then the allocation ratio arguments won't work.The various thing that the original code did for
ReferenceItemWithSizedoes not comply with this constraint, and therefore I changed the logic there, it will still create a linked list, but it will allocate exactly once.The python code is driven by the fact that the
GCPerfSimoutput is actually parsed by the infrastructure, so we need to make sure the infrastructure is aware of these extra outputs, and thenpy . lintdo all the other changes.