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

Further C++ improvement #49

Open
chainsawriot opened this issue Sep 28, 2023 · 4 comments
Open

Further C++ improvement #49

chainsawriot opened this issue Sep 28, 2023 · 4 comments
Assignees

Comments

@chainsawriot
Copy link
Collaborator

          yeah. I am eyeing on removing all of these

https://github.com/schochastics/adaR/blob/14692c751ac2fdbc97caa5b491357788d51eb7f6/R/parse.R#L20-L42

Originally posted by @chainsawriot in #47 (comment)

@chainsawriot chainsawriot self-assigned this Sep 28, 2023
@chainsawriot
Copy link
Collaborator Author

@schochastics We can set this for v0.3. HEAD is feature complete and safe.

@chainsawriot chainsawriot changed the title Make recode happen in C++ Further C++ improvement Sep 28, 2023
@chainsawriot
Copy link
Collaborator Author

chainsawriot commented Sep 28, 2023

@chainsawriot
Copy link
Collaborator Author

chainsawriot commented Sep 28, 2023

valgrind output of devtools::test()

==3850610== 
==3850610== HEAP SUMMARY:
==3850610==     in use at exit: 124,925,892 bytes in 95,588 blocks
==3850610==   total heap usage: 731,838 allocs, 636,250 frees, 654,821,627 bytes allocated
==3850610== 
==3850610== LEAK SUMMARY:
==3850610==    definitely lost: 760,087 bytes in 14,028 blocks
==3850610==    indirectly lost: 294,324 bytes in 6,872 blocks
==3850610==      possibly lost: 6,433 bytes in 98 blocks
==3850610==    still reachable: 123,865,048 bytes in 74,590 blocks
==3850610==                       of which reachable via heuristic:
==3850610==                         newarray           : 4,264 bytes in 1 blocks
==3850610==         suppressed: 0 bytes in 0 blocks
==3850610== Rerun with --leak-check=full to see details of leaked memory
==3850610== 
==3850610== For lists of detected and suppressed errors, rerun with: -s
==3850610== ERROR SUMMARY: 17908 errors from 2 contexts (suppressed: 0 from 0)
  • investigate which part is causing the leak?

@chainsawriot
Copy link
Collaborator Author

valgrind --leak-check=yes points to this line

https://github.com/schochastics/adaR/blob/c9939674de621989a39af267e920e31d6f8dfb49/src/adaR.cpp#L6

Maybe we should new and delete it?

schochastics pushed a commit that referenced this issue Sep 28, 2023
* Fix leak

valgrind indicates no leak anymore; but we have the remaining issue of `charsub` with "Invalid read of size 1"

* Fix valgrind invalid read

But might have performance impact; need to check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants