Lazy loading: load static initializers for needed classes. Fixes #846 #855
Conversation
|
@smowton could you please take a look at the overly long lines reported? |
| \*******************************************************************/ | ||
|
|
||
| #include "ci_lazy_methods.h" | ||
| #include <string> |
There was a problem hiding this comment.
Nit picking: STL headers first, blank line, local include
| #include "ci_lazy_methods.h" | ||
| #include <string> | ||
|
|
||
| void ci_lazy_methodst::add_needed_method(const irep_idt &method_symbol_name) |
There was a problem hiding this comment.
Function comment headers missing
src/java_bytecode/ci_lazy_methods.h
Outdated
|
|
||
| class ci_lazy_methodst | ||
| { | ||
| public: |
There was a problem hiding this comment.
Single-character indent - no indent needed for public/private/protected
src/java_bytecode/ci_lazy_methods.h
Outdated
|
|
||
| private: | ||
| std::vector<irep_idt> &needed_methods; | ||
| std::set<irep_idt> &needed_classes; |
There was a problem hiding this comment.
Why is one of those a vector while the other is a set?
smowton
force-pushed
the
fix/lazy_methods_and_clinit
branch
3 times, most recently
from
94832db to
5bbec11
Compare
|
Amended and added comments to address your question about vector vs. set. |
| // needed_classes on the other hand is a true set of every class | ||
| // found so far, so we can use a membership test to avoid | ||
| // repeatedly exploring a class hierarchy. | ||
| std::set<irep_idt> &needed_classes; |
There was a problem hiding this comment.
Thanks a lot for the clarification; I'd suggest to use std::unordered_set<irep_idt, irep_id_hash> &needed_classes as a minor improvement.
There was a problem hiding this comment.
I'd want to make change a few others too, so I'll do that in a subsequent PR
| { | ||
| if(!needed_classes.insert(class_symbol_name).second) | ||
| return false; | ||
| const irep_idt &clinit_name=id2string(class_symbol_name)+".<clinit>:()V"; |
There was a problem hiding this comment.
What's the reason for taking a const ref to a temporary here vs. the following?
const irep_idt clinit_name(id2string(class_symbol_name)+".<clinit>:()V");
smowton
force-pushed
the
fix/lazy_methods_and_clinit
branch
from
5bbec11 to
286fcc5
Compare
This adds a wrapper for the needed-methods and needed-classes objects used to accumulate dependencies during lazy loading, such that the first time a class is noted needed, its static initializer if any is also queued for elaboration.
The test case is borrowed from @reuk.