[
https://issues.apache.org/jira/browse/WICKET-6774?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17097287#comment-17097287
]
Thomas Heigl edited comment on WICKET-6774 at 5/1/20, 10:37 AM:
----------------------------------------------------------------
Hi [~papegaaij]:
{quote}We should be very careful with these micro benchmarks. Your numbers
already show a very interesting case: your testcase contains an error on
component2. It is constructed with the same parameters as component0, still the
numbers are way off. Either your actual testcase is somewhat different or the
difference in numbers has a whole different cause. It can be a difference in
memory allocation, but it can also be that your system is throttling down due
to heat or power consumption. Interestingly enough, I'm seeing similar numbers.
I don't have a good explanation.
{quote}
I might have temporarily fixed that mistake and then later changed it back when
switching between branches. I frequently stashed and reverted to older versions
of the benchmark during my experiments. But that does not explain why you see
similar numbers. I have no explanation either.
{quote}More importantly, we are only testing detach. I'm not saying that detach
performance can be increased, but this case is very limited. During detach of
your testcase, {{setMetaData}} is called many times with data = null. This is
the only call to any of the state of the components. Therefore, it makes sense
that in this particular case performance is limited by this call, and
eliminating it will increase performance for this case. However, this is not a
typical application and the other elements of the state are accessed by other
parts of Wicket. Optimizing for this case may reduce performance for the other
(probably more important) cases.
That being said, I do think we should continue profiling and benchmarking
component state, but in a richer setup. I've continued to improve the
performance of my branch. For the detach case, I'd consider it on par with or
faster than master.
{quote}
I agree. Detach shouldn't be the main test-case. I only created the benchmark
this way to demonstrate a performance issue I saw in the profiler on
production. This was not intended to be a performance test that can be used to
fine-tune component state management.
{quote}I think we should add the feedback flag. The setMetaData call per
component is a lot more expensive than a simple flag check. Not many components
do have feedback.
{quote}
Either that or a general {{FLAG_METADATA_SET}} would be the simplest
performance improvement that can be made to the current implementation. The
general flag has the advantage that it can be used to return early when
clearing other metadata. MarkupContainer clears removals on every detach by
setting the metadata to null:
{code:java}
@Override
protected void onDetach()
{
super.onDetach();
removals_clear();
}
private void removals_clear()
{
setMetaData(REMOVALS_KEY, null);
}
{code}
If we had {{FLAG_METADATA_SET}}, {{setMetaData}} could return immediately like
this:
{code:java}
@Override
public final <M extends Serializable> Component setMetaData(final
MetaDataKey<M> key, final M object)
{
if (!getFlag(FLAG_METADATA_SET) && object == null)
{
return this;
}
...
}
{code}
was (Author: thomas.heigl):
Hi [~papegaaij]:
{quote}We should be very careful with these micro benchmarks. Your numbers
already show a very interesting case: your testcase contains an error on
component2. It is constructed with the same parameters as component0, still the
numbers are way off. Either your actual testcase is somewhat different or the
difference in numbers has a whole different cause. It can be a difference in
memory allocation, but it can also be that your system is throttling down due
to heat or power consumption. Interestingly enough, I'm seeing similar numbers.
I don't have a good explanation.
{quote}
I might have temporarily fixed that mistake and then later changed it back when
switching between branches. I frequently stashed and reverted to older versions
of the benchmark during my experiments. But that does not explain why you see
similar numbers. I have no explanation either.
{quote}More importantly, we are only testing detach. I'm not saying that detach
performance can be increased, but this case is very limited. During detach of
your testcase, {{setMetaData}} is called many times with data = null. This is
the only call to any of the state of the components. Therefore, it makes sense
that in this particular case performance is limited by this call, and
eliminating it will increase performance for this case. However, this is not a
typical application and the other elements of the state are accessed by other
parts of Wicket. Optimizing for this case may reduce performance for the other
(probably more important) cases.
That being said, I do think we should continue profiling and benchmarking
component state, but in a richer setup. I've continued to improve the
performance of my branch. For the detach case, I'd consider it on par with or
faster than master.
{quote}
I agree. Detach shouldn't be the main test-case. I only created the benchmark
this way to demonstrate a performance issue I saw in the profiler on
production. This was not intended to be a performance test that can be used to
fine-tune component state management.
{quote}I think we should add the feedback flag. The setMetaData call per
component is a lot more expensive than a simple flag check. Not many components
do have feedback.
{quote}
Either that or a general {{FLAG_METADATA_SET}} would be the simplest
performance improvement that can be made to the current implementation. The
general flag has the advantage that it can be used to return early when
clearing other metadata. MarkupContainer clears removals on every detach by
setting the metadata to null:
{code:java}
@Override
protected void onDetach()
{
super.onDetach();
removals_clear();
}
private void removals_clear()
{
setMetaData(REMOVALS_KEY, null);
}
{code}
If we had {{FLAG_METADATA_SET, }}{{setMetaData}} could return immediately like
this:
{code:java}
@Override
public final <M extends Serializable> Component setMetaData(final
MetaDataKey<M> key, final M object)
{
if (!getFlag(FLAG_METADATA_SET) && object == null)
{
return this;
}
...
}
{code}
> Separate model, behaviors and metadata into separate fields
> -----------------------------------------------------------
>
> Key: WICKET-6774
> URL: https://issues.apache.org/jira/browse/WICKET-6774
> Project: Wicket
> Issue Type: Improvement
> Components: wicket-core
> Affects Versions: 9.0.0-M5
> Reporter: Thomas Heigl
> Priority: Major
> Attachments: benchmarks.png
>
>
> While investigating performance issues with metadata in WICKET-6771, I
> discovered that significant performance gains can be achieved by separating
> models, behaviors, and metadata into separate fields.
> Currently, all three types of data are stored in a single, untyped field
> {{Component.data}}. The idea is to minimize memory overhead by creating as
> few objects as possible.
> If a model or a single behavior or metadata is added, {{data}} stores only a
> reference to the object. When additional data is added, the reference becomes
> an array.
> This is the most memory-efficient way to store these three types of data. But
> it comes with a cost: code to manipulate that data structure is complex and
> not as efficient because it has to take all possible combinations of data
> into account.
> I suggest introducing 3 separate fields for the 3 types of data, trading a
> little bit of memory for reduced complexity and performance gains.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
