[jira] [Commented] (WICKET-6776) Cache Urls returned from ServletWebRequest
[
https://issues.apache.org/jira/browse/WICKET-6776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17113183#comment-17113183
]
Martin Tzvetanov Grigorov commented on WICKET-6776:
---
We can deprecate them even after 9.0.0 is released. Let's see what the new API
will look like so we know what to use as a deprecation message.
> Cache Urls returned from ServletWebRequest
> --
>
> Key: WICKET-6776
> URL: https://issues.apache.org/jira/browse/WICKET-6776
> Project: Wicket
> Issue Type: Improvement
> Components: wicket-core
>Affects Versions: 8.7.0, 9.0.0-M5
>Reporter: Thomas Heigl
>Priority: Major
>
> Profiling in production showed that {{ServletWebRequest.getUrl()}} and
> {{ServletWebRequest.getClientUrl()}} are called many times during request
> processing and allocate a lot of objects.
> Each call to {{ServletWebRequest.getUrl()}} creates a new {{Url}} object that
> internally creates two new {{ArrayLists}}.
> {{ServletWebRequest.getClientUrl()}} creates a new URL and involves a lot of
> complex parsing logic.
> Both methods are invoked during request mapping. We have about 350 registered
> mappers, so these methods are called *at least* 350 times on each request.
> Both values should be cached.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (WICKET-6776) Cache Urls returned from ServletWebRequest
[
https://issues.apache.org/jira/browse/WICKET-6776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17112192#comment-17112192
]
Thomas Heigl commented on WICKET-6776:
--
[~mgrigorov]: Should we deprecate the setter methods on {{Url}} before Wicket 9
is released, so we can remove them in Wicket 10?
> Cache Urls returned from ServletWebRequest
> --
>
> Key: WICKET-6776
> URL: https://issues.apache.org/jira/browse/WICKET-6776
> Project: Wicket
> Issue Type: Improvement
> Components: wicket-core
>Affects Versions: 8.7.0, 9.0.0-M5
>Reporter: Thomas Heigl
>Priority: Major
>
> Profiling in production showed that {{ServletWebRequest.getUrl()}} and
> {{ServletWebRequest.getClientUrl()}} are called many times during request
> processing and allocate a lot of objects.
> Each call to {{ServletWebRequest.getUrl()}} creates a new {{Url}} object that
> internally creates two new {{ArrayLists}}.
> {{ServletWebRequest.getClientUrl()}} creates a new URL and involves a lot of
> complex parsing logic.
> Both methods are invoked during request mapping. We have about 350 registered
> mappers, so these methods are called *at least* 350 times on each request.
> Both values should be cached.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (WICKET-6776) Cache Urls returned from ServletWebRequest
[
https://issues.apache.org/jira/browse/WICKET-6776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100535#comment-17100535
]
Thomas Heigl commented on WICKET-6776:
--
{quote}I'd like Url class itself to be immutable but as I suggested at dev@ we
should release 9.0.0 soon and stop making breaking changes. Otherwise it will
pass another year.
{quote}
I fully agree. I've been waiting for Wicket 9 for many months now ;)
{quote}The best we can do now is your approach with ImmutableUrl. Just make
sure it is private so we can get rid of it for Wicket 10
{quote}
I'll create a PR for Wicket 10 once Wicket 9 is released. It probably isn't
worth to add it now and remove it again later.
> Cache Urls returned from ServletWebRequest
> --
>
> Key: WICKET-6776
> URL: https://issues.apache.org/jira/browse/WICKET-6776
> Project: Wicket
> Issue Type: Improvement
> Components: wicket-core
>Affects Versions: 8.7.0, 9.0.0-M5
>Reporter: Thomas Heigl
>Priority: Major
>
> Profiling in production showed that {{ServletWebRequest.getUrl()}} and
> {{ServletWebRequest.getClientUrl()}} are called many times during request
> processing and allocate a lot of objects.
> Each call to {{ServletWebRequest.getUrl()}} creates a new {{Url}} object that
> internally creates two new {{ArrayLists}}.
> {{ServletWebRequest.getClientUrl()}} creates a new URL and involves a lot of
> complex parsing logic.
> Both methods are invoked during request mapping. We have about 350 registered
> mappers, so these methods are called *at least* 350 times on each request.
> Both values should be cached.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (WICKET-6776) Cache Urls returned from ServletWebRequest
[
https://issues.apache.org/jira/browse/WICKET-6776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100103#comment-17100103
]
Martin Tzvetanov Grigorov commented on WICKET-6776:
---
I like the idea of caching those!
I also understand that the returned Url should be immutable.
I'd like Url class itself to be immutable but as I suggested at dev@ we should
release 9.0.0 soon and stop making breaking changes. Otherwise it will pass
another year.
The best we can do now is your approach with ImmutableUrl. Just make sure it is
private so we can get rid of it for Wicket 10
> Cache Urls returned from ServletWebRequest
> --
>
> Key: WICKET-6776
> URL: https://issues.apache.org/jira/browse/WICKET-6776
> Project: Wicket
> Issue Type: Improvement
> Components: wicket-core
>Affects Versions: 8.7.0, 9.0.0-M5
>Reporter: Thomas Heigl
>Priority: Major
>
> Profiling in production showed that {{ServletWebRequest.getUrl()}} and
> {{ServletWebRequest.getClientUrl()}} are called many times during request
> processing and allocate a lot of objects.
> Each call to {{ServletWebRequest.getUrl()}} creates a new {{Url}} object that
> internally creates two new {{ArrayLists}}.
> {{ServletWebRequest.getClientUrl()}} creates a new URL and involves a lot of
> complex parsing logic.
> Both methods are invoked during request mapping. We have about 350 registered
> mappers, so these methods are called *at least* 350 times on each request.
> Both values should be cached.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (WICKET-6776) Cache Urls returned from ServletWebRequest
[
https://issues.apache.org/jira/browse/WICKET-6776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17099681#comment-17099681
]
Thomas Heigl commented on WICKET-6776:
--
Shall I create a PR for this? I have deployed this to production and it works
fine, but returning immutable URLs might theoretically break other users
applications.
> Cache Urls returned from ServletWebRequest
> --
>
> Key: WICKET-6776
> URL: https://issues.apache.org/jira/browse/WICKET-6776
> Project: Wicket
> Issue Type: Improvement
> Components: wicket-core
>Affects Versions: 8.7.0, 9.0.0-M5
>Reporter: Thomas Heigl
>Priority: Major
>
> Profiling in production showed that {{ServletWebRequest.getUrl()}} and
> {{ServletWebRequest.getClientUrl()}} are called many times during request
> processing and allocate a lot of objects.
> Each call to {{ServletWebRequest.getUrl()}} creates a new {{Url}} object that
> internally creates two new {{ArrayLists}}.
> {{ServletWebRequest.getClientUrl()}} creates a new URL and involves a lot of
> complex parsing logic.
> Both methods are invoked during request mapping. We have about 350 registered
> mappers, so these methods are called *at least* 350 times on each request.
> Both values should be cached.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (WICKET-6776) Cache Urls returned from ServletWebRequest
[
https://issues.apache.org/jira/browse/WICKET-6776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17096256#comment-17096256
]
Thomas Heigl commented on WICKET-6776:
--
I solved this in my application by extending {{ServletWebRequest}} like this:
{code}
public class BaseWebRequest extends ServletWebRequest {
private Url clientUrl;
private Url url;
public BaseWebRequest(HttpServletRequest httpServletRequest, String
filterPrefix) {
super(httpServletRequest, filterPrefix);
}
@Override
public Url getClientUrl() {
if (clientUrl == null) {
clientUrl = new ImmutableUrl(super.getClientUrl());
}
return clientUrl;
}
@Override
public Url getUrl() {
if (url == null) {
url = new ImmutableUrl(super.getUrl());
}
return url;
}
static final class ImmutableUrl extends Url {
public ImmutableUrl(Url url) {
super(url);
}
@Override
public void setFragment(String fragment) {
throw new UnsupportedOperationException();
}
@Override
public void setQueryParameter(String name, Object value) {
throw new UnsupportedOperationException();
}
@Override
public void setProtocol(String protocol) {
throw new UnsupportedOperationException();
}
@Override
public void setContextRelative(boolean contextRelative) {
throw new UnsupportedOperationException();
}
@Override
public void setPort(Integer port) {
throw new UnsupportedOperationException();
}
@Override
public void setHost(String host) {
throw new UnsupportedOperationException();
}
}
}
{code}
Urls are mutable so I'm putting an immutable wrapper around them to guard
against accidental changes.
> Cache Urls returned from ServletWebRequest
> --
>
> Key: WICKET-6776
> URL: https://issues.apache.org/jira/browse/WICKET-6776
> Project: Wicket
> Issue Type: Improvement
> Components: wicket-core
>Affects Versions: 8.7.0, 9.0.0-M5
>Reporter: Thomas Heigl
>Priority: Major
>
> Profiling in production showed that {{ServletWebRequest.getUrl()}} and
> {{ServletWebRequest.getClientUrl()}} are called many times during request
> processing and allocate a lot of objects.
> Each call to {{ServletWebRequest.getUrl()}} creates a new {{Url}} object that
> internally creates two new {{ArrayLists}}.
> {{ServletWebRequest.getClientUrl()}} creates a new URL and involves a lot of
> complex parsing logic.
> Both methods are invoked during request mapping. We have about 350 registered
> mappers, so these methods are called *at least* 350 times on each request.
> Both values should be cached.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
