playing about with simpler buffer copying #1841
Conversation
|
👋 Welcome back david-beaumont! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
| } | ||
| assert(cfs != nullptr, "must be able to read the classfile data of shared classes for built-in loaders."); | ||
| log_debug(aot, jvmti)("classfile data for %s [%d: %s] = %d bytes", class_name, path_index, | ||
| log_debug(aot, jvmti)("classfile data for %s [%d: %s] = %d bytes", file_name, path_index, |
There was a problem hiding this comment.
Assuming (hoping) that the exact format of this log is not expected to be important to anyone.
| // Include terminating nul. | ||
| int name_len = symbol->utf8_length() + 1; | ||
| char* const buffer = NEW_RESOURCE_ARRAY(char, name_len + padding_len); | ||
| symbol->as_C_string(buffer, name_len); |
There was a problem hiding this comment.
Avoids intermediate copy as happened in previous code.
| } | ||
|
|
||
| // caller needs ResourceMark | ||
| const char* ClassLoader::file_name_for_class_name(const Symbol* class_name) { |
There was a problem hiding this comment.
While it's undocumented in the code, I'm assuming that the symbol name is <module>/path/to/ClassName (e.g. "java.base/java/lang/Integer").
This is because all the existing code does is append ".class" to the end.
| const char* const file_name = file_name_for_class_name(name); | ||
| assert(file_name != nullptr, "invariant"); | ||
|
|
||
| EventMarkClassLoading m("Loading class %s", file_name); |
There was a problem hiding this comment.
I'm not 100% sure what's going on here, so someone needs to check that this looks sane.
| static const char* file_name_for_class_name(const char* class_name, | ||
| int class_name_len); | ||
|
|
||
| static char* symbol_name_to_padded_buffer(const Symbol* symbol, int padding_len); |
There was a problem hiding this comment.
I should probably declare this private ...
1 participant
Not a real pull request yet, just experimenting with simplifying buffer copying.
Progress
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/1841/head:pull/1841$ git checkout pull/1841Update a local copy of the PR:
$ git checkout pull/1841$ git pull https://git.openjdk.org/valhalla.git pull/1841/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1841View PR using the GUI difftool:
$ git pr show -t 1841Using diff file
Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/1841.diff