Skip to content
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

CFE ES unnecessarily keeping multiple copies of task/app names #502

Closed
jphickey opened this issue Jan 30, 2020 · 1 comment · Fixed by #946 or #959
Closed

CFE ES unnecessarily keeping multiple copies of task/app names #502

jphickey opened this issue Jan 30, 2020 · 1 comment · Fixed by #946 or #959
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

Is your feature request related to a problem? Please describe.
When reviewing the changes for other tickets, noted that the CFE ES is storing the app name in the AppTable twice. It is stored in the StartParams.Name subfield, as well as the TaskInfo.MainTaskName sub field. These appear to be always set together, to the same value, such as in CFE_ES_AppCreate for example:

strncpy((char *)CFE_ES_Global.AppTable[i].StartParams.Name, AppName, OS_MAX_API_NAME);

strncpy((char *)CFE_ES_Global.AppTable[i].TaskInfo.MainTaskName, AppName, OS_MAX_API_NAME);

It then goes on to store the same string a third time in the TaskTable[x].TaskName field for the task itself:

strncpy((char *)CFE_ES_Global.TaskTable[TaskId].TaskName,
(char *)CFE_ES_Global.AppTable[i].TaskInfo.MainTaskName,OS_MAX_API_NAME );

The name is a string value and therefore takes a fair bit of memory to store. In the "stock" example config this bloats the size of the ES data structures by about 2500 bytes, but could easily be much more in a real deployment if OS_MAX_API_NAME is set longer and/or the max number of apps/tasks is larger.

Describe the solution you'd like
Should store at most one copy of the name in the AppTable, but even that might not be needed if it is always the same as the main task name (which it appears to be).

For tasks, OSAL already stores the task name. For the ES API calls that need to get the name, it should just lookup the name from OSAL, just like we are doing for SB in #404

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@CDKnightNASA CDKnightNASA self-assigned this Apr 4, 2020
@CDKnightNASA
Copy link
Contributor

In fact, we don't need MainTaskName, since we have MainTaskID and it can be found by looking it up in the task table, plus the Task table has the AppName but we can also find this as the Task table has the AppId as well. So a couple cleanups will decrease the memory footprint of ES.

jphickey added a commit to jphickey/cFE that referenced this issue Oct 14, 2020
There is no need to store the main task name a second time in
the app record, as it is already stored in the task record.
@jphickey jphickey linked a pull request Oct 14, 2020 that will close this issue
@jphickey jphickey self-assigned this Oct 14, 2020
jphickey added a commit to jphickey/cFE that referenced this issue Oct 14, 2020
There is no need to store the main task name a second time in
the app record, as it is already stored in the task record.
astrogeco added a commit that referenced this issue Oct 21, 2020
Fix #943, #924 and #502, improve resource ID management
@astrogeco astrogeco added this to the 7.0.0 milestone Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants