[VL] Separate each jni wrapper to different files #2845

Activity

1. Yohahaha

   added

   enhancementNew feature or requestNew feature or request

   on Aug 22, 2023
2. Yohahaha commented on Sep 11, 2023

   

   Yohahaha

   on Sep 11, 2023

   ContributorAuthor

   More actions

   After issue 3077 completed, I will separate current JniWrapper.cc into different sub-dir, it will looks like below:

   cpp
     |- jni
     |- memory
     |- compute
       |- r2c
         |- R2CJniWrapper.cc
         |- Row2Columnar.h
         |- test
         |- benchmark
       |- c2r
         |- C2RJniWrapper.cc
         |- Columnar2Row.h
         |- ...
       |- shuffle
         |- ShuffleReaderJniWrapper.cc
         |- ShuffleReader.h
         |- ShuffleWriterJniWrapper.cc
         |- ShuffleWriter.h
         |- ...
       |- serializer
       |- wholestage
       |- [common files]
     |- ...
   

   Any suggestions? @jinchengchenghh @zhztheplayer @rui-mo
3. zhztheplayer commented on Sep 12, 2023

   

   zhztheplayer

   on Sep 12, 2023

   Member

   More actions

   This sort of refactor is needed.

   The thing is that we should find a proper time to apply the change, since it may easily conflict with other pending PRs.
4. Yohahaha commented on Sep 12, 2023

   

   Yohahaha

   on Sep 12, 2023

   ContributorAuthor

   More actions

   The thing is that we should find a proper time to apply the change, since it may easily conflict with other pending PRs.

   Good point, I prefer do the refactor in weekend, before that, let's discuss the structure we expected.
5. Yohahaha commented on Sep 12, 2023

   

   Yohahaha

   on Sep 12, 2023

   ContributorAuthor

   More actions

   Also cc @ulysses-you @FelixYBW @zhouyuan
6. ulysses-you commented on Sep 12, 2023

   

   ulysses-you

   on Sep 12, 2023

   Contributor

   More actions

   I think make jni file name align with java class is good. The VexloJniWrapper should also be taken into account. e.g., The java class PlanEvaluatorJniWrapper has several jni methods that map to both JniWrapper and VexloJniWrapper. What's the really difference between VexloJniWrapper and JniWrapper ?
7. zhztheplayer commented on Sep 13, 2023

   

   zhztheplayer

   on Sep 13, 2023

   Member

   More actions

   The java class PlanEvaluatorJniWrapper has several jni methods that map to both JniWrapper and VexloJniWrapper.

   PlanEvaluatorJniWrapper -> VeloxJniWrapper

   Such mappings should be finally eliminated IIUC. We already had Backend and ExecutionCtx as the APIs across backends.
8. github-actions

   added

   stalestalestale

   on Nov 17, 2023
9. github-actions

   closed this as not plannedon Nov 25, 2023
10. weiting-chen

    added this to Gluten 1.1.0on Nov 30, 2023