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

.NET NORM Extension #85

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

.NET NORM Extension #85

wants to merge 24 commits into from

Conversation

mullerj
Copy link
Contributor

@mullerj mullerj commented Mar 28, 2024

Added a .NET extension that provides a .NET wrapper so that .NET applications can access the NORM C API. It is modeled after the Java extension for NORM. Also, added integration tests to verify functionality and demonstrate usage. Added GitHub actions to run tests and publish the .NET extension to GitHub Packages. Included Dev Container to simplify the process of setting up a development environment. This closes #84.

mullerj and others added 16 commits February 9, 2023 11:49
@mullerj @SergiyYermak @Slyfree3
.NET Extension Design (#5)
3a20a8b 
* Designed .NET classes from Java

Co-authored-by: Sergiy <sergiy.yermak.ctr@us.navy.mil>

* Added NormApi static class

* Started on api uml

Co-authored-by: mullerj <mullerj@users.noreply.github.com>
Co-authored-by: Sylvester Freeman III <Slyfree3@users.noreply.github.com>

* Add static methods to api uml

* add static method to normapi uml

* Add enums

* Changes

* Overload methods with defaults

* type marshalling

* change handles to in nuint

* marshall more handles

* Removed in/ref and reformatted.

Co-authored-by: mullerj <mullerj@users.noreply.github.com>

* Removed duplicate enums

* Removed NormEvent from NormApi

* Fixed typo

* design NormInstance methods

Co-authored-by: mullerj <mullerj@users.noreply.github.com>

* Fixed CreateInstance method

* Changed handles to long

* Change id types to long

* Remove condition checks that do nothing

* Removed UserData functions

* Removed NormAlloc() and NormFree()

* Cleaned up some API methods

* Fix misspelling

Co-authored-by: Sylvester Freeman III <Slyfree3@users.noreply.github.com>

* Design NormSession methods

Co-authored-by: Sylvester Freeman III <Slyfree3@users.noreply.github.com>

* Added missing comma

* Uml for enqueue
Co-authored-by: mullerj <mullerj@users.noreply.github.com>

* Updated design based on enqueue test

* Updated EnqueueFile sequence diagram

Co-authored-by: Sylvester Freeman III <Slyfree3@users.noreply.github.com>

* Moved sequence diagrams

* Added constants to NormApi

* Removed method level sequence diagrams

---------

Co-authored-by: Sergiy <sergiy.yermak.ctr@us.navy.mil>
Co-authored-by: syermak <syerm001@odu.edu>
Co-authored-by: mullerj <mullerj@users.noreply.github.com>
Co-authored-by: Sylvester Freeman III <Slyfree3@users.noreply.github.com>
Co-authored-by: Sergiy <47069243+SergiyYermak@users.noreply.github.com>
Co-authored-by: Sylvester Freeman III <Slyfreeman3@gmail.com>
@mullerj @Slyfree3 @SergiyYermak
.NET Extension Design Implementation (#10)
2694446 
* changes

* Removed GetsNextEvent test

* Implemented GetNextEvent

* Fixed NormStream design

* Fixed HasNextEvent design

* Added NormGetNextEvent overload

* Implemented HasNextEvent

* Updated design

* Added Kernel32 to design

* Modified to use .NET EventWaitHandle instead of kernel32 P/Invoke

* Modified to only copy norm.dll on Windows

* Added copy for non Windows

* add start and stop receiver

* add receiver

* Modified non-Windows to use Socket for HasNextEvent

* Deleted NormFileTests since they break the other tests

* Fixed ReceiverReceives test

* Updated ReceiverReceives test name

* Modified ReceivesFile to get all events instead of adding to array

* Cleaned up NormInstance formatting

* Refactored tests

* Updated NormFile

* Implemented send stream

* Implemented receive stream

* Implemented NormFile Name

* Updated ReceivesFile to use events

* data enqueue

* Fixed merge

* Fixed ReceivesData test

* Modified data test to compare collections without order

* stop instance working

* enqueues data

* Reverted upgrade to .NET 7

* Restartinstance is working

* Modified NormData to read bytes instead of as string

* Removed using statements in NormData

* Updated design

* Updated design

* Switched to nint

* Switched design to nint

* unfinished test for suspendinstance

* working suspend instance

* added resume function to instance

* Updated session send and receive data tests

* Removed launch settings

* session configuration functions

* Modified to use random port due to intermittent failed tests due to session conflict

* Modified to randomize text content

* Implemented NormNode Address

* Updated design for NormNode

* Implemented NormNode Grtt

* Implemented NormNode Id

* implement session configuration functions

* instance

* missing return types

* add missing handle param

* normNode

* Implemented RequeueObject

* Implemented SetWatermark and AckingNode

* Implemented CancelWatermark

* Implemented ResetWatermark

* Implemented SendCommand

* Implemented CancelCommand

* Implemented DebugLevel. Removed DebugPipe

* Updated SetTxPort

* Updated SetRxPortReuse

* Implemented SetEcnSupport

* Added test for RxLoss

* Implemented ReportInterval

* Refactored session calculated properties

* Updated enqueue methods

* Updated StreamOpen design

* Fixed SetWatermark overload

* Implemented GetAckingStatus

* Removed SendCommand overload

* Removed SetSilentReceiver overload

* Implemented NormEvent ToString

* Implemented NormFile Rename

* Implemented SetUnicastNack

* Added ReceivesCommand test

* Implemented SetNackingMode

* Implemented SetRepairBoundary

* Added Fact to SetsRepairBoundary test

* Implemented SetRxRobustFactor

* Implemented NormNode Command

* Implemented FreeBuffers

* Implemented Retain and Release

* Fix using namespaces

* test get object type

* NormObject get size

* NormObject setNackingMode

* NormObject get bytes pending

* NormObjet cancel

* NormObject retain

* NormObject release

* NormObject get sender

* NormObject get hashcode

* normObject equals

* Implemented NormObject Equals

* Removed redundant cast

* Removed unused using directive

* Removed static using

* Added NormApi to global Usings

* Added Enums to global usings

* Cleaned up NormObject Equals

* Cleaned up NormObject GetHashCode() test

* Cleaned up warning

* normStream has vacancy

* normStream read offset

* normStream seek msg start

* NormEventListener interface

* normStream set push enable

* normStream set auto flush

* Simplified NormApi usage

* Updated design of NormStream

* public interface

* Fixed INormEventListener

* fixing name space

* commit initializing  inputstream

* Added StreamBreakException

* Fixed formatting

* Fixed formatting

* Fixed formatting

* Initial implementation of NormOutputStream

* Cleaned up NormObject Equals

* NormInputStream class

Co-authored-by: mullerj <mullerj@users.noreply.github.com>

* normStream data offset

* move offset length arithmetic into write method

* building normInputstream

* all method implemented

Co-authored-by: mullerj <mullerj@users.noreply.github.com>

* NormStreamRead

* Fixed build error

* Completed ReceivesStreamWithOffset test

* Added retries to GetsCommand test

* Updated type for NormStreamRead

* Simplified NormStream Read

* Simplified NormStream Write

* Added dataOffset to DataEnqueue

* Fixed DataEnqueue overload

* Added infoOffset to FileEnqueue

* Added infoOffset to StreamOpen

* Updated design

* Removed extra overload of FileEnqueue

* Removed DataEnqueue overloads

* Removed extra overload from StreamOpen

* Added HasNextEvent overload

* Implemented OpenDebugPipe

* add thread locking

* locking for properties

* Not supported exceptions

* Implementing sychronize

* fixing inputstream

* adding sychronize message

* Reorganized NormApi and updated design

* Modified to skip GetsCommand test on IO exception

* WIP: 537b706 adding sychronize message

* adding message to expections

* Fixed test file cleanup

* Added locking to NormSession methods that use static field

* Changed StartsSender to skippablefact

* Changed to SetsNackingMode to skippable fact

* Converted tests to skippable

* Converted more tests to skippable

* Converted all tests that start sender to skippable

* Updated README about skipped tests

* documentation

* 2 doc

* Cleaned up input and output stream classes

* Cleaned up NormApi

* Updated NormOutputStream Write

* Added document comments to NormApi

* Added comments to NormApi

* Removed unnecessary usings

* Added comments to NormApi

* set up the rest of doc

* api documentation

* fix typo

* fix spacing

* Doc for instance need clean up

* clean up

* document norm session

* NormStream documentation

* commit for node

* NormObject docs

* Updated NormApi comments

* Cleaned up NormInstance comments

* Cleaned up NormNode comments

* Cleaned up NormObject comments

* Cleaned up NormSession comments

* Cleaned up NormStream comments

* Cleaned up NormObject comments

* Adding commits

* Update get commit

* clean up on data and event

* Norm Data change

* clean up normfile docs

* clean up normstream

* clean up api docs

* Fixed data

* space approved

* Cleaned up NormData comment

* documentation clean up

* Update the ReadMe

* Cleaned up comments

* Added comments to NormAckingStatus

* Added comments to NormEventType

* Added comments to NormFecType

* Added comments to NormNackingMode

* Added comments to NormObjectType

* Added comments to NormProbingMode

* Added comments to NormRepairBoundary

* Added comments to NormFlushMode

* Added comments to NormSyncPolicy

* Cleaned up comments.  Fixed default for StartSender

* Added comments to constants

* Moved constants to relevant classes

* Updated design

* Removed NormDataDetachData since it is not referenced

* Updated design

* Updated input and output stream design

* Updated packaging README

* Removed language version since redundant

* Cleaned up project file

* Updated to package from the dotnet directory (test project is set to not pack)

* Added version to project

---------

Co-authored-by: syermak <syerm001@odu.edu>
Co-authored-by: Sylvester Freeman III <Slyfreeman3@gmail.com>
Co-authored-by: Sergiy <47069243+SergiyYermak@users.noreply.github.com>
Co-authored-by: mullerj <mullerj@users.noreply.github.com>
@mullerj
NormSession typos in comments (#12)
701360a 
* Fixed default function comments in NormSession

* Fixed typos in NormSession

* Fixed default function comments
@mullerj
Feature/9 net norm extension target architecture packages (#13)
db20c64 
* Added Win64 project and release configuration

* Added project reference to Win64 project

* Added Win32 project. Moved norm build actions to scripts

* Added linux .NET extension project

* Modified linux project to copy to output directory

* Fixed linux pack script

* Changed Linux project to copy always

* Changed Linux project package path

* Moved Windows libraries to runtimes path

* Renamed runtime specific projects
@mullerj
Merge branch 'USNavalResearchLaboratory:master' into master
86d8fb1
@mullerj
Codespace Configurations (#15)
96476ff 
* Created devcontainer.json
@mullerj
.NET norm extension GitHub actions (#16)
a1dcbf1 
* Added .NET action
@mullerj
Update dotnet-publish.yml to use source branch (#18)
e60a3af
@mullerj
.NET norm package publish (#19)
47bf91a 
* Added steps to copy package and check workspace

* Changed trigger for testing

* Removed if statement

* Updated dotnet-publish.yml to copy to  $GITHUB_OUTPUT

* Changed from variable to path

* Removed copy command

* Modified to check runner directory

* Added step to publish package

* Added package write permission

* Added job to publish win-x64

* Fixed package win-x64 step

* Added job to publish win-x86

* Added skip-duplicate to publish commands

* Added publish job

* Changed to . for linux find

* Added workflow trigger

* Added branches filter
@mullerj
Publish test results (#21)
7d7de23 
* Added step to publish test results

* Changed upload step to use wildcard

* Added quotes

* Changed directory to relative path

* Fixed upload step

* Added step to publish test results

* Added permissions
@mullerj
Dev Container extensions (#23)
2d8768c 
* Added extensions

* Cut down on extensions

* Added extension

* Removed pull requests extension

* Added workspace recommendation

* Added powershell feature

* Fixed powershell version

* Updated powershell version and added extension

* Updated to latest powershell
@mullerj
Dev Container performance (#25)
2be146c 
* Fixed devcontainer format

* Added dotnet-linux-x64 devcontainer

* Changed devcontainer image to python

* Pinned OS version

* Updated devcontainer base image

* Reverted changes to devcontainer

* Moved linux devcontainer file

* Updated dotnet-publish to use subfolder
@mullerj
Revert "Dev Container performance (#25)" (#27)
43c0736 
This reverts commit 2be146c.
@mullerj
Merge branch 'USNavalResearchLaboratory:master' into master
b04acd3
@mullerj
Update action versions for dotnet-publish.yml
034980e
@mullerj
Update action versions for dotnet.yml
b5d2e87
@mullerj mullerj changed the title .NET NORM Extension #84 .NET NORM Extension Mar 28, 2024
@bebopagogo
Copy link
Collaborator

bebopagogo commented Mar 28, 2024 via email

@mullerj
Copy link
Contributor Author

mullerj commented Mar 28, 2024

Great! Please let me know if you have any questions or comments.

lbargaoanu
lbargaoanu reviewed May 27, 2024
View reviewed changes
src/dotnet/src/Mil.Navy.Nrl.Norm/NormApi.cs Outdated
/// NORM_INFO content is left to the application's discretion</param>
/// <returns>A NormObjectHandle is returned which the application may use in other NORM API calls as needed.</returns>
[DllImport(NORM_LIBRARY)]
public static extern long NormDataEnqueue(long sessionHandle, byte[] dataPtr, int dataLen, byte[]? infoPtr, int infoLen);
Copy link

@lbargaoanu lbargaoanu May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A managed buffer won't work so well here. Managed data can move in memory and NORM needs to reference the data after the call to Enqueue. Forcing the users of this wrapper to do the pinning seems harsh :) One option is to have callers use unmanaged memory by passing an IntPtr or byte*. Another would be to have the wrapper handle the pinning/unpinning transparently.

Copy link
Contributor Author

@mullerj mullerj May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point about using a managed buffer versus using unmanaged memory. The managed buffer also does not follow the "Native interoperability best practices" of using the .NET type that maps closest to the native type. When using a source generator with LibraryImport, IntPtr only generates a single extern method whereas byte[] generates additional logic for handling the managed buffer.

Updating the NormDataEnqueue extern method to use IntPtr and adding a wrapper method that takes the managed buffer and handles the pinning/unpinning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lbargaoanu Please review updated NormDataEnqueue extern method and wrapper method to verify that the new approach addresses the comment.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid that won't really work. It would work if NORM were to copy the user buffer inside Enqueue. But it doesn't, it stores the pointer and reads from it when it needs to. So it's safe to unpin the buffer only when we know that NORM doesn't need it anymore. When the object is purged, when the send is cancelled and when the sender is stopped. Not sure whether the purged event is fired when the sender is stopped or if there are other interesting cases.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe cancel triggers the object purged notification:

norm/src/common/normApi.cpp

Line 2697 in 2ae69e6

instance->PurgeObjectNotifications(objectHandle);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lbargaoanu Thanks for the feedback. SafeBuffer seems to be the best solution for NormSession.DataEnqueue(). It solves the pinning/unpinning issue and uses a .NET Standard class that is fairly consistent with the Java extension. NormApi.NormDataEnqueue() can use IntPtr and NormSession.DataEnqueue() can use built-in SafeBuffer functionality to obtain the pointer to pass to NormApi.NormDataEnqueue().

Can include an example of using .NET Standard classes for working with unmanaged memory,MemoryMappedFile and SafeMemoryMappedViewHandle, and passing the SafeBuffer to NormSession.DataEnqueue(). Can also add an example of using a custom class derived from SafeBuffer that can be used similar to Java ByteBuffer.

Copy link

@lbargaoanu lbargaoanu Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't notice that all the generic SafeBuffer implementations are internal. SafeMemoryMappedViewHandle is too specialized and we don't need to bring memory mapped files in the conversation :) So maybe just stick with a raw pointer. In the end, the user doesn't have to keep track of this buffer, NORM can provide it.
When you have a small buffer, you can work with a managed array and then copy it to unmanaged memory. For larger buffers, you could work directly with the unmanaged buffer with UnmanagedMemoryAccessor or UnmanagedMemoryStream. Here is a concrete generic SafeBuffer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for providing the link to the concrete SafeBuffer. The .NET NORM extension can include a similar class. Agree about SafeMemoryMappedViewHandle being too specialized so it does not have to be included as an example. Might want to use SafeBuffer for the NormSession since it provides useful functionality for working with unmanaged memory and provides flexibility, such as working with UnmanagedMemoryStream. NormApi can use a raw pointer.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe having both is best. For example, UnmanagedMemoryStream has constructors for both. The thing is, when using a SafeBuffer, the user has to keep a reference to a buffer, otherwise it could be finalized and the memory freed while NORM is still using it. You can work with a pointer without keeping track because you can get it from NORM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point about having both. Can just overload the NormSession.DataEnqueue() method.

@mullerj
NormData Refactor (#30)
3cdb415 
* Switched NormData Data property to a method

* Corrected NormData summary comment

* Updated NormData GetData to use Marshal.Copy

* Fixed warnings

* Updated NormData design
@mullerj
NormDataEnqueue Refactor (#31)
edbcff7 
* Switched NormDataEnqueue to use IntPtr

* Updated NormApi design
@bebopagogo
Copy link
Collaborator

Yes - the NormDataEnqueue() call pass the pointer to the application memory block and the application is responsible for keeping it intact and unmodified while the NORM sender has access to it. This is sort of an anachronism from the early days of our reliable multicast effort where we were focused on transfer of "bulky" content and when passinng very large objects in memory and NORM keep a potentially large cache of objects to provide repair/retransmissions for, it wasn't prudent for NORM to keep an internal copy of something the application was maintaining anyway. For smaller data object like messages, it is practical for NORM to maintain its own copy of the data and I have thought about allowing for that. In fact, the API may have a way to pass deallocation control down to NORM (if not, an option that could be added with appropriate use of accompanying NormAlloc() and NormDealloc() calls).

BTW - also posting here that I have been tracking this pull request loosely. Since there is some additional engagement on this and I've been busy, I haven't yet dived into closely review this request. If you both thinks it's ready for that, I will do that ASAP. Thanks for your contribution!

@lbargaoanu
Copy link

Having NORM copy on enqueue and deallocate when needed would certainly simplify things for the .NET code. Otherwise we're left with pinning or doing the exact same thing in C#. The performance impact of pinning seems difficult to estimate without measuring a real use case. But it negatively affects GC performance due to fragmentation and it would increase with the size of the transmit cache.

@mullerj
Copy link
Contributor Author

mullerj commented Jun 4, 2024

@bebopagogo Thanks for checking on the pull request and for clarifying NormDataEnqueue(). I am currently working on a feature branch to address the NormDataEnqueue() comments. The rest could use further review if time is available.

mullerj and others added 5 commits June 5, 2024 22:17
@mullerj
Updated GetSession to use lock
b9c18c5
@mullerj
Updated GetSession to TryGetValue
0662326
@mullerj
DataEnqueue Refactor (#33)
662e1fa 
* Refactored DataEnqueue to use pointer arithmetic
@mullerj
Merge branch 'USNavalResearchLaboratory:master' into master
0eb0b82
@mullerj
NormDataEnqueue Refactor (#34)
d539a35 
* Updated to use SafeBuffer

* Added ByteBuffer and DirectByteBuffer

* Added pointer overload to DataEnqueue

* Updated FileEnqueue() to use pointer arithmetic

* Updated NormStream to use pointer arithmetic

* Updated NormSendCommand to nint

* Updated NormNodeGetCommand to nint

* Updated NormNodeGetAddress to use nint for buffer

* Updated NormStreamRead to use nint for buffer

* Updated NormObjectGetInfo to use nint for buffer

* Updated NormFileGetName nameBuffer to nint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lbargaoanu lbargaoanu lbargaoanu left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Suggestion: .NET NORM Extension
3 participants
@mullerj @bebopagogo @lbargaoanu