Skip to content

[Tools] Replace in memap CSV report fields #2161

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

Merged
merged 1 commit into from
Jul 18, 2016
Merged

[Tools] Replace in memap CSV report fields #2161

merged 1 commit into from
Jul 18, 2016

Conversation

PrzemekWirkus
Copy link
Contributor

@PrzemekWirkus PrzemekWirkus commented Jul 13, 2016

Changes

  • This change is clean-up after [Tools] Add summary for test building #2047 (Add memory usage summary for test building)
  • In code of memory usage summary columns were shortened and names changed a bit,
    see here
    • Change was done to make columns not so wide for user.
    • This change did not propagate to JSON and CSV reports generated by memap script.
  • There is a need to align memap CSV report fields (summary columns) with what
    is in code now. This discrepancy may create problems when generating reports
    in the future. All columns should have the same name.
  • memap CSV summary fields names changes:
    • total_static_ram -> static_ram
    • allocated_heap -> heap
    • allocated_stack -> stack

See example of discrepancy:

  • total_static_ram vs static_ram
  • allocated_heap vs heap
  • allocated_stack vs stack

Current implementation outputs

What is in code and presented with prettytable:

+----------------------------------------+--------+-----------+------------+-------+-------+-----------+-------------+
| name                                   | target | toolchain | static_ram | stack |  heap | total_ram | total_flash |
+----------------------------------------+--------+-----------+------------+-------+-------+-----------+-------------+
| tests-mbed_drivers-c_strings           | K64F   | GCC_ARM   |      11468 | 32768 | 65540 |    109776 |       74182 |
| tests-mbed_drivers-callback            | K64F   | GCC_ARM   |      11980 | 32768 | 65540 |    110288 |       88406 |
| tests-mbedmicro-rtos-mbed-threads      | K64F   | GCC_ARM   |      11468 | 32768 | 65540 |    109776 |       75918 |
| tests-mbedmicro-rtos-mbed-timer        | K64F   | GCC_ARM   |      10788 | 32768 | 65540 |    109096 |       68534 |
+----------------------------------------+--------+-----------+------------+-------+-------+-----------+-------------+

vs CSV:

total_static_ram,allocated_heap,allocated_stack,total_ram,total_flash
10788,65540,32768,109096,67486

vs memstats_map.JSON:

[
    {
        "module": "Fill",
        "size": {
            ".data": 4,
            ".bss": 2257,
            ".text": 173
        }
    },

    {
        "summary": {
            "total_flash": 67486,
            "allocated_stack": 32768,
            "total_static_ram": 10788,
            "total_ram": 109096,
            "allocated_heap": 65540
        }
    }
]

Changes:
* This change impacts #2047
* In code summary columns were shortened and names changed a bit,
  see [here](https://github.com/mbedmicro/mbed/pull/2047/files?w=1#diff-f9cb941bde5647a5763e18fc220efc51R410)
* There is a need to allign memap CSV report fields (summary columns) with what
  is in code now. This discrapency may create problems when generating reports
  in the future. All columns should have the same name.
* memap CSV sumamry fields names changes:
  'total_static_ram' -> 'static_ram'
  'allocated_heap' -> 'heap'
  'allocated_stack' -> 'stack'

See example of discrepancy:

'total_static_ram' vs 'static_ram'
'allocated_heap' vs 'heap'
'allocated_stack' vs 'stack'

What is in code and presented with prettytable:

+----------------------------------------+--------+-----------+------------+-------+-------+-----------+-------------+
| name                                   | target | toolchain | static_ram | stack |  heap | total_ram | total_flash |
+----------------------------------------+--------+-----------+------------+-------+-------+-----------+-------------+
| tests-mbed_drivers-c_strings           | K64F   | GCC_ARM   |      11468 | 32768 | 65540 |    109776 |       74182 |
| tests-mbed_drivers-callback            | K64F   | GCC_ARM   |      11980 | 32768 | 65540 |    110288 |       88406 |
| tests-mbedmicro-rtos-mbed-threads      | K64F   | GCC_ARM   |      11468 | 32768 | 65540 |    109776 |       75918 |
| tests-mbedmicro-rtos-mbed-timer        | K64F   | GCC_ARM   |      10788 | 32768 | 65540 |    109096 |       68534 |
+----------------------------------------+--------+-----------+------------+-------+-------+-----------+-------------+

vs CSV:

```
total_static_ram,allocated_heap,allocated_stack,total_ram,total_flash
10788,65540,32768,109096,67486
```

vs memstats_map.JSON:

```json
[
    {
        "module": "Fill",
        "size": {
            ".data": 4,
            ".bss": 2257,
            ".text": 173
        }
    },

    {
        "summary": {
            "total_flash": 67486,
            "allocated_stack": 32768,
            "total_static_ram": 10788,
            "total_ram": 109096,
            "allocated_heap": 65540
        }
    }
]
```
@PrzemekWirkus
Copy link
Contributor Author

After change example

Memory map breakdown for built projects (values in Bytes):
+-----------------------------------+--------+-----------+------------+-------+-------+-----------+-------------+
| name                              | target | toolchain | static_ram | stack |  heap | total_ram | total_flash |
+-----------------------------------+--------+-----------+------------+-------+-------+-----------+-------------+
| tests-integration-threaded_blinky | K64F   | GCC_ARM   |      10788 | 32768 | 65540 |    109096 |       68430 |
+-----------------------------------+--------+-----------+------------+-------+-------+-----------+-------------+

vs CSV:

static_ram,heap,stack,total_ram,total_flash
10788,65540,32768,109096,68430

vs JSON

[
    {
        "module": ".build/tests", 
        "size": {
            ".data": 92, 
            ".bss": 4063, 
            ".text": 27798
        }
    }, 
    {
        "summary": {
            "total_flash": 68430, 
            "static_ram": 10788, 
            "stack": 32768, 
            "heap": 65540, 
            "total_ram": 109096
        }
    }
] 

@PrzemekWirkus
Copy link
Contributor Author

@bridadan @screamerbg @mlnx Please review
@adbridge @mazimkhan @0xc0170 FYI

@theotherjimmy
Copy link
Contributor

👍 I like the diff, very clean, very clear.

@bridadan
Copy link
Contributor

Thanks for the thorough description Przemek!

+1

@PrzemekWirkus
Copy link
Contributor Author

@0xc0170 We can try to merge this :)

@0xc0170 0xc0170 merged commit 3ea625c into ARMmbed:master Jul 18, 2016
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.

4 participants