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.

Sign up for GitHub

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

Jump to bottom

Integration Candidate 2020-10-27 #975

Merged
merged 10 commits into from
Oct 28, 2020
Merged

Integration Candidate 2020-10-27 #975

merged 10 commits into from
Oct 28, 2020

Conversation

astrogeco
Copy link
Contributor

@astrogeco astrogeco commented Oct 28, 2020 •
edited
Loading

Describe the contribution
Fix #28
Fix #173
Fix #928
Fix #929
Fix #950

Testing performed
See Bundle CI: https://github.com/nasa/cFS/pull/153/checks

Expected behavior changes

  • PR Fix #28, provide library API #960

    • Provide Library API similar to App API
    • Allows the existing CFE_ES_AppInfo_t structure to be extended to libraries as well as applications by introducing a new value (3) for the Type field.
    • Allows Libraries to be queried via API calls similar to App API.
    • Extends the Query All/Query One commands to operate on Libraries or Applications.
    • Breaks up the monolithic AppCreate and LoadLibrary functions and have these call subroutines that operate on the common components.
    • Fix race conditions in app request processing state machine.

  • PR Fix #928 and #929 - Modularize software bus routing, add msg map hash #947

    • Adds SBR module which includes message map and routing table. The access APIs are on the SB side which still owns the destination logic
    • removes passing of route index or pointers being. Everything is now based on route and message id
    • Oversized the hash message map (4x) to minimize collisions
    • added debug event for collisions during add
    • dropped routing push/pop, dropped "key" in direct implementation
    • hash designed for 32 bit, a change in CFE_SB_MsgId_Atom_t size may require implementation updates
    • Deletes unused code CFE_SB_FindGlobalMsgIdCnt
    • Fixes variable declaration violations of coding standard
    • Individual events for deleting destinations when deleting a pipe removed to avoid a race condition
    • around a 10-20% performance hit to hash via rough comparison on a linux box, no memory impact

System(s) tested on

  • Hardware: [e.g. PC, SP0, MCP750]
  • OS: [e.g. Ubuntu 18.04, RTEMS 4.11, VxWorks 6.9]
  • Versions: [e.g. cFE 6.6, OSAL 4.2, PSP 1.3 for mcp750, any related apps or tools]

Additional context
Part of nasa/cFS#153

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
@skliper
@jphickey

skliper and others added 9 commits October 21, 2020 09:36
@skliper
Fix #928, Extracted msg map and route table from SB
33364e8 
- Removed all index/pointer accesses of message map and routing table
- All references now via IDs and APIs
- Note SB still owns destination logic (unchanged linear linked list)
- Limited whitespace fixes for readability
- Resolved observed instances of variables not declared at the
  start of functions
- Cleaned comments
- Resolved potential double locks in CFE_SB_SendPrevSubs
- Route and message write to file no longer guaranteed in msgid
  order to maintain performance for large msgid space implementations
- Removed unused CFE_SB_FindGlobalMsgIdCnt
- Clarified CFE_PLATFORM_SB_MAX_MSG_IDS config param description
- Eliminated potential race in CFE_SB_PipeDeleteFull
- Individual destination removal debug events no longer reported
  during a CFE_SB_PipeDeleteFull
@skliper
Fix #928, Added software bus routing module
0aac36a 
- Implementation for direct map and unsorted routing table
- Includes full coverage tests
- Removed msg key and route stack concepts from direct map
@skliper
Fix #929, Add message map has implementation
5a795ad 
- Message map size based on used routes
- Oversized (4x) to limit collisions while retaining
  resonable size related to routing table (still smaller)
- ~10% single collisions seen for full routing table
  with realistic message ID use
- Oversizing means map can never fill, simplifies logic
- Observed approximately 10%-20% performance hit, trade
  against memory use (can now use full 32 bit MsgId space)
- Hash intended for 32 bit, if CFE_SB_MsgId_Atom_t size
  changes may require modification to hash
- Also added full coverage unit tests
@skliper
Fix #928, Update SB unit test for SBR module with direct link
9b56d52 
- Linking SBR with SB unit test, not stubbed
- Confirms matching functionality (with updates for
  intended changes)
@astrogeco
Merge pull request #947 from skliper/fix928-route-direct-module
12f2ff1 
Fix #928 and #929 - Modularize software bus routing, add msg map hash
@jphickey
Fix #950, race condition in control request
6932f1f 
Because the process of handling a control request involves
calling other subsystems, the ES lock needs to be released.
However, this also means that the app record can change state
for other reasons, such as the app self-exiting at the same
time.

To avoid this possibility, process in two phases:

First assemble a list of tasks that have timed out
and need to be cleaned up, while ES is locked.

Next actually perform the cleanup, while ES is unlocked.
In areas during cleanup that need to update the ES global,
the lock is locally re-acquired and released.
@jphickey
Fix #173, refactor app and lib loading procedure
0749d69 
Reorganize the global data structures for apps and
libraries into components that can be shared between
the two concepts.

Break up the monolithic AppCreate and LoadLibrary
functions and have these call subroutines that
operate on the common components.
@jphickey
Fix #28, Provide Library API similar to App API
1c75d33 
Allows the existing "CFE_ES_AppInfo_t" structure to be extended to
libraries as well as applications by introducing a new value (3)
for the Type field.

Allows Libraries to be queried via API calls similar to App API.

Also extends the Query All/Query One commands to operate on
Libraries or Applications.
@astrogeco
Merge pull request #960 from jphickey/fix-28-app-lib-api
35f3b03 
Fix #28, provide library API
@astrogeco astrogeco mentioned this pull request Oct 28, 2020
Merged
@astrogeco astrogeco merged commit e0d1ce8 into main Oct 28, 2020
astrogeco added a commit to nasa/cFS that referenced this pull request Oct 28, 2020
@astrogeco
Integration Candidate 2020-10-27 (#153)
0c4f5d8 
Includes nasa/cFE#975
@skliper skliper added this to the 7.0.0 milestone Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Caelum
3 participants
@astrogeco @skliper @jphickey