Issue663

Title use shared_ptr for searches
Priority feature Status resolved
Superseder Nosy List danielk, jendrik, malte
Assigned To danielk Keywords
Optional summary

Created on 2016-08-04.17:07:32 by jendrik, last changed by jendrik.

Files
File name Uploaded Type Edit Remove
issue663-experiment.patch jendrik, 2017-05-01.18:14:05 text/x-patch
Messages
msg6392 (view) Author: jendrik Date: 2017-06-06.11:12:07
Merged and pushed. Thanks, Daniel!
msg6391 (view) Author: malte Date: 2017-06-04.19:51:52
It it passes all tests and the option parser/output still looks good afterwards,
it looks good to me. The buildbot will check both of these, so feel free to merge.
msg6377 (view) Author: jendrik Date: 2017-05-18.22:14:26
The code looks good to me. Malte, can we 
merge this?
msg6376 (view) Author: danielk Date: 2017-05-18.17:27:14
All tests have passed when using shared_ptr for all SearchEngines.
Pullrequest:
https://bitbucket.org/danielkillenberger/fast-downward/pull-requests/7/return-
shared_ptr-for-searchengine/diff
msg6312 (view) Author: malte Date: 2017-05-02.11:45:07
Sounds good.
msg6307 (view) Author: jendrik Date: 2017-05-01.18:14:05
I made a quick experiment and converted the "astar" and "iterated" plugins to 
shared_ptr. This seems to be easier than anticipated. Since the search plugins are 
currently worked on in issue721, I only post the experimental patch and we can 
continue working on this after issue721 is merged.
msg6197 (view) Author: jendrik Date: 2017-04-19.16:16:46
Thanks for the warning. We'll defer working on this issue and tackle a different 
issue instead.
msg6196 (view) Author: malte Date: 2017-04-19.15:54:35
One obstacle towards moving shared_ptr for searches is that searches -- and as
far as I can recall only searches -- currently use a low-level string-based
parsing method for re-generating new objects based on the command line later on
during their execution. This is used (only) by iterated search.

The clean solution towards getting rid of this is making the option parser
create factories rather than actual search engines, similar to the way this is
already done for option parser, but unfortunately I think this can only be done
properly if the same is done for all other objects that searches directly or
indirectly hold on to, which is more or less all objects creatable from the
command line.

I'm not sure how easy it is to switch searches to shared_ptr in general before
first resolving this other issue. Perhaps the low-level method from the option
parser could easily be changed to return a shared_ptr, perhaps not. I don't know
where else it is used internally in the option parser.
msg5482 (view) Author: jendrik Date: 2016-08-04.17:07:32
Once the parser returns shared pointers to search engines we should make sure 
that the iterated search doesn't hold on to old searches.
History
Date User Action Args
2017-06-06 11:12:07jendriksetstatus: reviewing -> resolved
messages: + msg6392
2017-06-04 19:51:52maltesetmessages: + msg6391
2017-05-18 22:14:26jendriksetmessages: + msg6377
2017-05-18 17:27:14danielksetstatus: chatting -> reviewing
assignedto: danielk
messages: + msg6376
2017-05-02 11:45:07maltesetmessages: + msg6312
2017-05-01 18:14:05jendriksetfiles: + issue663-experiment.patch
messages: + msg6307
2017-04-19 16:16:46jendriksetstatus: in-progress -> chatting
assignedto: danielk -> (no value)
messages: + msg6197
2017-04-19 15:54:35maltesetmessages: + msg6196
2017-04-19 15:33:28danielksetstatus: unread -> in-progress
nosy: + danielk
assignedto: danielk
2016-08-04 17:07:32jendrikcreate