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

Optimize memory usage of wifi scan for REALTEK_RTW8195AM #4863

Merged
merged 2 commits into from Aug 14, 2017

Conversation

Projects
None yet
6 participants
@Archcady
Contributor

Archcady commented Aug 7, 2017

To prevent new and delete frequently, use placement new to create instance at local array.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Aug 7, 2017

cc @pan-

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Aug 7, 2017

Looks good to me. One question, with this allocation during scanning. I see ap_num to be set to 0, means it always starts 0 and allocates new wifi objects all over again. Not needed to destroy the allocated objects, no clean-up?

@Archcady

This comment has been minimized.

Contributor

Archcady commented Aug 8, 2017

The allocations here create new WiFiAccessPoint instance at address of ap_details, and ap_detail actually is passed in as first paramerter res in RTWInterface::scan(WiFiAccessPoint *res, unsigned count). So there's no need to delete ap_details here.

@pan-

This comment has been minimized.

Member

pan- commented Aug 8, 2017

@Archcady Thanks for this precision, that change the fix needed a bit: It is not a chunk of uninitialized memory which is passed to the function then used. scan_handler->ap_details is actually an array of initialized WiFiAccessPoint instances.

In such case, placement new is not the right approach, instead you can just use the copy assignment operator:

scan_handler->ap_details[scan_handler->ap_num] = WiFiAccessPoint(ap);

Just to give a bit more of context, usage of one or another depends on the type of memory and the type of instance to initialize:

  • If the memory is not initialized and therefore untyped: use placement new and release it with an explicit call to the destructor. Beware of alignment constraints when allocating/reserving the memory block: new (memory_block) TypeToConstruct(...)

  • If an instance has to be replaced by another one and the copy assignment operator is accessible. Use the copy assignment operator: old_instance = TypeToConstruct(...).

  • If an instance has to be replaced by another one and the copy assignment operator is not accessible. Destroy explicitly the existing instance then use placement new to insert the new one:

// explicit destructor call on old_instance reference
old_instance.~TypeToConstruct(); 
// construct a new instance at old_instance place
new (&old_instance) TypeToConstruct(...)
@Archcady

This comment has been minimized.

Contributor

Archcady commented Aug 10, 2017

@pan- Thank you very much! I tested both approaches and they all pass the test. Despite that, I think using copy assignment is better.

@0xc0170 0xc0170 removed the needs: review label Aug 10, 2017

@0xc0170 0xc0170 added the needs: CI label Aug 10, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Aug 10, 2017

/morph test

@Archcady Archcady changed the title from Use placement new to optimize wifi scan for REALTEK_RTW8195AM to Optimize memory usage of wifi scan for REALTEK_RTW8195AM Aug 11, 2017

@studavekar

This comment has been minimized.

Collaborator

studavekar commented Aug 11, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Aug 11, 2017

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1008

Test failed!

@mbed-bot

This comment has been minimized.

mbed-bot commented Aug 11, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1014

All builds and test passed!

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Aug 11, 2017

@theotherjimmy theotherjimmy merged commit 9607441 into ARMmbed:master Aug 14, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Archcady Archcady deleted the Archcady:RTWInf branch Aug 15, 2017

exmachina-auto-deployer pushed a commit to exmachina-dev/mbed-os that referenced this pull request Sep 13, 2017

Merge tag 'mbed-os-5.5.6' of github.com:ARMmbed/mbed-os into 5.5.6
mbed OS 5.5.6 release

We are pleased to announce the [mbed OS 5.5.6
release](https://github.com/ARMmbed/mbed-os/releases/tag/mbed-os-5.5.6)
is now available.

This release includes ...

Known Issues

The following list of known issues apply to this release:

Contents

Ports for Upcoming Targets

[4608](ARMmbed#4608)
Support Nuvoton's new target NUMAKER_PFM_M487

[4840](ARMmbed#4840)
Add Support for TOSHIBA TMPM066 board

Fixes and Changes

[4801](ARMmbed#4801)
STM32 CAN: Fix issue with speed function calculation

[4808](ARMmbed#4808)
Make HAL & US tickers idle safe

[4812](ARMmbed#4812)
Use DSPI SDK driver API's in SPI HAL driver

[4832](ARMmbed#4832)
NUC472/M453: Fix several startup and hal bugs

[4842](ARMmbed#4842)
Add call to DAC_Enable as this is no longer done as part

[4849](ARMmbed#4849)
Allow using of malloc() for reserving the Nanostack's heap.

[4850](ARMmbed#4850)
Add list of defines to vscode exporter

[4863](ARMmbed#4863)
Optimize memory usage of wifi scan for REALTEK_RTW8195AM

[4869](ARMmbed#4869)
HAL LPCs SPI: Fix mask bits for SPI clock rate

[4873](ARMmbed#4873)
Fix Cortex-A cache file

[4878](ARMmbed#4878)
STM32 : Separate internal ADC channels with new pinmap

[4392](ARMmbed#4392)
Enhance memap, and configure depth level

[4895](ARMmbed#4895)
Turn on doxygen for DEVICE_* features

[4817](ARMmbed#4817)
Move RTX error handlers into RTX handler file

[4902](ARMmbed#4902)
Using CMSIS/RTX Exclusive access macro

[4923](ARMmbed#4923)
fix export static_files to zip

[4844](ARMmbed#4844)
bd: Add ProfilingBlockDevice for measuring higher-level applications

[4896](ARMmbed#4896)
target BLUEPILL_F103C8 compile fix

[4921](ARMmbed#4921)
Update gcc-arm-embedded PPA in Travis

[4926](ARMmbed#4926)
STM32L053x8: Refactor NUCLEO_L053R8 and DISCO_L053C8 targets

[4831](ARMmbed#4831)
Remove excessive use of printf/scanf in mbed_fdopen/_open

[4922](ARMmbed#4922)
bug fix: xdot clock config

[4935](ARMmbed#4935)
STM32: fix F410RB vectors size

[4940](ARMmbed#4940)
Update mbed-coap to version 4.0.9

[4941](ARMmbed#4941)
Update of MemoryPool.h header file.

You can fetch this release from the [mbed-os
GitHub](https://github.com/ARMmbed/mbed-os) repository,
using the tag "mbed-os-5.5.6".

Please feel free to ask any questions or provide feedback on this
release [on the forum](https://forums.mbed.com/),
or to contact us at [support@mbed.org](mailto:support@mbed.org).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment