CodefoundryDE / LegacyWrapper

LegacyWrapper uses a x86 wrapper to call legacy dlls from a 64 bit process (or vice versa).
MIT License
79 stars 20 forks source link

Performance issue #36

Closed 110-kenichi closed 3 years ago

110-kenichi commented 3 years ago

Calling API is too heavy. Are there any tips?

zalintyre commented 3 years ago

Hello,

what do you mean by "too heavy"? Too slow? This is always relative ;) Can you provide some measurements?

Kind regards Franz

110-kenichi commented 3 years ago

Hi,

Too slow... Caller my app is 64bit and Callee DLL is 32bit. The DLL is transmitting data via Serial port. But, I don't sure what the DLL is doing.

Can you provide some measurements?

Do I just provide the "Native 32bit to 32bit calling time AND 64bit to 32 bit wrapper calling time" ? Please wait for a while...

Thanks

110-kenichi commented 3 years ago

Bottleneck code

Stopwatch stopwatch = Stopwatch.StartNew(); wrapperClient.SetRegister(pChip, dAddr, pData); //TOO SLOW stopwatch.Stop(); timespan += stopwatch.Elapsed;

32 to 32 w/ native : 0.0033695s

https://user-images.githubusercontent.com/40394569/104835613-2b2f2180-58eb-11eb-9a29-e350952c71f3.mp4

64 to 32 w/ wrapper : 22.9436008s

https://user-images.githubusercontent.com/40394569/104835618-2ff3d580-58eb-11eb-8c5c-e36530324ee5.mp4

zalintyre commented 3 years ago

Hello,

I'm afraid your code snippet provide not enough information to reproduce your problem.

Can you provide:

The two measured times imply an increase of computation time with a factor 6800. Because LegacyWrapper runs the same code for 32bit and 64bit, it is more likely that this issue is caused either by your (operating) system or your hardware.

Kind regards Franz

110-kenichi commented 3 years ago

Hi,

Hmm, I would like to confirm you before profiling... you have not a performance issue on your environment?

Thanks

110-kenichi commented 3 years ago

Hi,

SetRegister() API is a simple VC++ API.

declspec(dllexport) BOOL stdcall SetRegister(void pChip, DWORD dAddr, DWORD dData) { if (pChip == NULL) return FALSE; return ((SoundChip )pChip)->setRegister(dAddr, dData); }

And, marshaling API is here.

[LegacyDllImport("ScciWrapper.dll")] public interface IScciWrapper : IDisposable { [LegacyDllMethod(CallingConvention = CallingConvention.StdCall)] void SetRegister(IntPtr pChip, uint dAddr, uint pData); }

I'm not sure the SoundChip->setRegister() API design. But I think there are no problem, because my 32bit app does not have any problem.

So, hot path is here in the SetRegister() API was described above.

image

image

How do you think?

I think you should not serialize primitive arguments of the SetRegister() API...?🤔 (If the wrappee API does not have a class/struct type argument, bypass serialization process)

zalintyre commented 3 years ago

Are you calling the DLL 5000 times in a second?

110-kenichi commented 3 years ago

Maybe YES.

Retro music player need to access sound control register only 1 byte at a time.

110-kenichi commented 3 years ago

Hi, almost resolved.

I used version 2.1 to strictly optimize. See the following optimization.

  1. Used the Delegate.Invoke() of the FastDelegate.Net lib instead of the Delegate.BeginInvoke()
  2. Cached the Delegate to invoke the Delegate.Invoke()
  3. Changed all API's parameters to primitive value parameters ( CallData.Type, CallData.Exception member type changed to String type )
  4. Used the ZeroFormatter lib instead of the BinaryFormatter. But, doesn't work... 😥

//If you optimize the latest version, could you please mention me ?

thanks,