8340409: [lworld] Simple serialization and deserialization of core migrated classes #1248
Conversation
…grated classes
Serialization and deerialization of value classes.
For java.base value classes being migrated from identity classes:
- Must be annotated with jdk.internal.MigratedValueClass (in tests too)
- Must have a constructor or static factory method:
- annotated with jdk.internal.value.DeserializeConstrucctor
- constructor parameters must match order and types of serializable fields of the class
- Value class must not have superclass with fields; only Object or abstract w/o fields
- Uses same technique as for Records to extract and concat values from stream to invoke constructor
- Migrated value classes are serialized using the same format/fields as the identity class
Non-migrated value classes throw InvalidClassException
Along the way, refactored code to break out different modes of deserialization:
full custom deserialization with custom data, default deserialization only,
externalizable, no local class, record, and then added mode to deserialize a value class.
Updates to value class tests and added a combo test to check many combinations
of value and identity classes with different parameters.
|
👋 Welcome back rriggs! A progress list of the required criteria for merging this PR into |
|
@RogerRiggs This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
|
@RogerRiggs this pull request can not be integrated into git checkout 8340409-simple-value-deserialize
git fetch https://git.openjdk.org/valhalla.git lworld
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge lworld"
git push |
| iae.printStackTrace(); | ||
| return null; | ||
| }}) | ||
| .filter(m -> matchFactoryParamTypes(clazz, m, fields)) |
There was a problem hiding this comment.
m can be null here, so matchFactoryParamTypes needs to check that.
There was a problem hiding this comment.
After setAccessible IAE should never happen; change to throw InternalError.
There was a problem hiding this comment.
Yeah, sounds good. 👍
Tidy up unreachable IllegalAccessException after setAccessible; throw InternalError. Move declaration of local into for loop header. Cleanup TRACE Property parsing. Improve test output readability.
|
Are there any more review comments; I'd like to get this integrated. |
|
/integrate |
|
@RogerRiggs Pushed as commit e02c628. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Serialization and deerialization of value classes.
For java.base value classes migrated from identity classes:
Non-migrated value classes throw InvalidClassException
Along the way, refactored code to break out different modes of deserialization: full custom deserialization with custom data, default deserialization only, externalizable, no local class, record, and then added mode to deserialize a value class.
Updates to value class tests and added a combo test to check many combinations of value and identity classes with different parameters.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/1248/head:pull/1248$ git checkout pull/1248Update a local copy of the PR:
$ git checkout pull/1248$ git pull https://git.openjdk.org/valhalla.git pull/1248/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1248View PR using the GUI difftool:
$ git pr show -t 1248Using diff file
Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/1248.diff
Webrev
Link to Webrev Comment