init: Stop combining actions
In the past, I had thought it didn't make sense to have multiple
Action classes with identical triggers within ActionManager::actions_,
and opted to instead combine these into a single action. In theory,
it should reduce memory overhead as only one copy of the triggers
needs to be stored.
In practice, this ends up not being a good idea.
Most importantly, given a file with the below three sections in this
same order:
on boot
setprop a b
on boot && property:true=true
setprop c d
on boot
setprop e f
Assuming that property 'true' == 'true', when the `boot` event
happens, the order of the setprop commands will actually be:
setprop a b
setprop e f
setprop c d
instead of the more intuitive order of:
setprop a b
setprop c d
setprop e f
This is a mistake and this CL fixes it. It also documents this order.
Secondly, with a given 'Action' now spanning multiple files, in order
to keep track of which file a command is run from, the 'Command'
itself needs to store this. Ironically to the original intention,
this increases total ram usage. This change now only stores the file
name in each 'Action' instead of each 'Command'. All in all this is a
negligible trade off of ram usage.
Thirdly, this requires a bunch of extra code and assumptions that
don't help anything else. In particular it forces to keep property triggers
sorted for easy comparison, which I'm using an std::map for currently,
but that is not the best data structure to contain them.
Lastly, I added the filename and line number to the 'processing
action' LOG(INFO) message.
Test: Boot bullhead, observe above changes
Test: Boot sailfish, observe no change in boot time
Change-Id: I3fbcac4ee677351314e33012c758145be82346e9
diff --git a/init/action.h b/init/action.h
index 0bae9f0..25e5a3e 100644
--- a/init/action.h
+++ b/init/action.h
@@ -27,31 +27,26 @@
#include "keyword_map.h"
class Command {
-public:
- Command(BuiltinFunction f, const std::vector<std::string>& args,
- const std::string& filename, int line);
+ public:
+ Command(BuiltinFunction f, const std::vector<std::string>& args, int line);
int InvokeFunc() const;
std::string BuildCommandString() const;
- std::string BuildSourceString() const;
-private:
+ int line() const { return line_; }
+
+ private:
BuiltinFunction func_;
std::vector<std::string> args_;
- std::string filename_;
int line_;
};
class Action {
-public:
- explicit Action(bool oneshot = false);
+ public:
+ explicit Action(bool oneshot, const std::string& filename, int line);
- bool AddCommand(const std::vector<std::string>& args,
- const std::string& filename, int line, std::string* err);
- void AddCommand(BuiltinFunction f,
- const std::vector<std::string>& args,
- const std::string& filename = "", int line = 0);
- void CombineAction(const Action& action);
+ bool AddCommand(const std::vector<std::string>& args, int line, std::string* err);
+ void AddCommand(BuiltinFunction f, const std::vector<std::string>& args, int line);
bool InitTriggers(const std::vector<std::string>& args, std::string* err);
bool InitSingleTrigger(const std::string& trigger);
std::size_t NumCommands() const;
@@ -60,11 +55,12 @@
bool CheckEventTrigger(const std::string& trigger) const;
bool CheckPropertyTrigger(const std::string& name,
const std::string& value) const;
- bool TriggersEqual(const Action& other) const;
std::string BuildTriggersString() const;
void DumpState() const;
bool oneshot() const { return oneshot_; }
+ const std::string& filename() const { return filename_; }
+ int line() const { return line_; }
static void set_function_map(const KeywordMap<BuiltinFunction>* function_map) {
function_map_ = function_map;
}
@@ -80,6 +76,8 @@
std::string event_trigger_;
std::vector<Command> commands_;
bool oneshot_;
+ std::string filename_;
+ int line_;
static const KeywordMap<BuiltinFunction>* function_map_;
};
@@ -115,18 +113,17 @@
};
class ActionParser : public SectionParser {
-public:
+ public:
ActionParser() : action_(nullptr) {
}
- bool ParseSection(const std::vector<std::string>& args,
+ bool ParseSection(const std::vector<std::string>& args, const std::string& filename, int line,
std::string* err) override;
- bool ParseLineSection(const std::vector<std::string>& args,
- const std::string& filename, int line,
- std::string* err) const override;
+ bool ParseLineSection(const std::vector<std::string>& args, int line, std::string* err) override;
void EndSection() override;
void EndFile(const std::string&) override {
}
-private:
+
+ private:
std::unique_ptr<Action> action_;
};