jsk-ros-pkg / jsk_pr2eus

PR2 euslisp packages
https://github.com/jsk-ros-pkg/jsk_pr2eus
4 stars 41 forks source link

[pr2eus] add provide in pr2-interface.l and pr2-utils.l #441

Closed knorth55 closed 3 years ago

knorth55 commented 4 years ago

add (provide :pr2-interface "pr2-interface.l")

Affonso-Gui commented 4 years ago

Is there something you really don't want to be loaded twice or is this just for code unification? If it's the latter, could you also add it on pr2-utils.l?

knorth55 commented 4 years ago

the former case, I mean. I just want to load this file once, but it doesn't accelerate loading files in my program that much.

Affonso-Gui commented 4 years ago

Not that I'm opposing this PR (I think it is good practice to add provides in files that are usually required), but I don't believe that loading pr2-interface multiple times is slowing your program down.

All of the files loaded from it are required, and ros::load-ros-manifest also have double-loading guards, so even if you load it twice not much harm is done.

irteusgl> (bench (load "package://pr2eus/pr2-interface.l"))
;; time -> 2.54901[s]
t
irteusgl> (bench (load "package://pr2eus/pr2-interface.l"))
;; time -> 0.019368[s]
t
knorth55 commented 4 years ago

yes, you are right. I don't see much improvement for loading pr2-interface.l multiple times. i make this pr to make the load faster but it doesn't affect much. but now i think it is better to have provide for formatting.

-- Shingo Kitagawa the University of Tokyo, JSK Lab s-kitagawa@jsk.imi.i.u-tokyo.ac.jp

2020年8月6日(木) 15:10 Affonso, Guilherme notifications@github.com:

Not that I'm opposing this PR (I think it is good practice to add provides in files that are usually required), but I don't believe that loading pr2-interface multiple times is slowing your program down.

All of the files loaded from it are required, and ros::load-ros-manifest also have double-loading guards, so even if you load it twice not much harm is done.

irteusgl> (bench (load "package://pr2eus/pr2-interface.l"));; time -> 2.54901[s]t irteusgl> (bench (load "package://pr2eus/pr2-interface.l"));; time -> 0.019368[s]t

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jsk-ros-pkg/jsk_pr2eus/pull/441#issuecomment-669723358, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACG6QX2GFG7WIT6XPQOYBT3R7JCMXANCNFSM4PWFA7IA .

Affonso-Gui commented 4 years ago

So... adding in pr2-utils.l as well?

knorth55 commented 4 years ago

ok i will

knorth55 commented 4 years ago

i updated and added pr2-utils.l