Bug 2028 – remove unnecessary Time to double conversions in wifi models

Bug 2028 - remove unnecessary Time to double conversions in wifi models


Summary:	remove unnecessary Time to double conversions in wifi models

Status:	RESOLVED FIXED

Product:	ns-3
Classification:	Unclassified
Component:	wifi
Version:	ns-3-dev
Hardware:	All All

Importance:	P5 normal
Assigned To:	sebastien.deronne

URL:
Keywords:

Depends on:
Blocks:
	Show dependency tree / graph

Reported:	2014-12-12 15:06 UTC by Tom Henderson
Modified:	2015-01-04 13:18 UTC (History)
CC List:	3 users (show)

See Also:

Attachments
patch to remove unnecessary Time to double conversions in wifi models (19.93 KB, patch) 2014-12-23 17:00 UTC, sebastien.deronne	Details \| Diff
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Tom Henderson 2014-12-12 15:06:31 UTC

This is a new bug for an issue discovered (and initially discussed) in the fix for bug 1969:  https://www.nsnam.org/bugzilla/﷒0﷓.

Some comments from Tommaso in bug 1969 follow:
---
Speaking of, it could be a wise idea to change ALL the functions to NnanoSeconds, i.e.:
- GetPlcpPreambleDurationMicroSeconds (payloadMode, preamble)
- GetPlcpHeaderDurationMicroSeconds (payloadMode, preamble)
- GetPlcpHtSigHeaderDurationMicroSeconds (payloadMode, preamble)
- GetPlcpHtTrainingSymbolDurationMicroSeconds (payloadMode, preamble,txvector)
- GetPayloadDurationMicroSeconds (size, txvector)
and maybe others.

Those should become:
- GetPlcpPreambleDurationNanoSeconds
etc.

If I'm right, the original code idea was to avoid altogether to use a double...

Speaking of... why GetPlcpPreambleDurationMicroSeconds ?

Shouldn't it be GetPlcpPreambleDuration and return a Time ?

WifiPhy::CalculateTxDuration is returning a Time. In that function a bunch of uint32_t are added, then a double (conversion to double), then a "Time" is created.

What is the performance hit to sum a bunch of Time variables instead ?

That would definitely make the code more foolproof and future-feature-proof.
----

so, the fix for this would be a patch that audits the code around this part of the codebase (WifiPhy) and removes unnecessary conversions to/from Time; may require some internal API changes...

Comment 1 sebastien.deronne 2014-12-13 04:13:56 UTC

Is someone already working on that or planned to work on that? 
If not, I may have a look at this issue.

Comment 2 Tommaso Pecorella 2014-12-13 11:40:38 UTC

I'd suggest to take a look at bug 1974 and see if the two can be harmonized.

Comment 3 sebastien.deronne 2014-12-23 17:00:24 UTC

Created attachment 1935 [details]
patch to remove unnecessary Time to double conversions in wifi models

I made a patch for this issue.

It turns the ...DurationMicroSeconds into ...Duration and now return a Time instead of a double. I updated Doxygen as well.

A good point is now that it removes the issues related to short guard intervals!

Note that I also added some additional unit tests in the devices-wifi-tx-duration test suite.

Comment 4 Tommaso Pecorella 2014-12-24 03:27:59 UTC

promoted to bug, since it can cause a real error,

+1 to push (need to rescan bindings).

Comment 5 Tom Henderson 2015-01-04 13:18:26 UTC

pushed in changeset 11119:067928a573cd