[GitHub] [wicket] theigl opened a new pull request #459: WICKET-6839 Use request flag for caching `isVisibleInHierarchy`

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [wicket] theigl opened a new pull request #459: WICKET-6839 Use request flag for caching `isVisibleInHierarchy`

GitBox

theigl opened a new pull request #459:
URL: https://github.com/apache/wicket/pull/459


   This PR aligns caching of `isVisibleInHierarchy` with `isEnabledInHierarchy`.
   
   We are currently clearing the flag `RFLAG_VISIBLE_IN_HIERARCHY_SET` every time the visibility of a component changes, but we never use the flag. This PR adds `RFLAG_VISIBLE_IN_HIERARCHY_VALUE` to cache the result of the computation.
   
   I had to change the type of `requestFlags` from `short` to `int` because all available 16 bits are already used up. If my understanding of the JVM memory layout is correct, this should *not* increase memory usage because all object fields use at least one slot of 32 bits anyways (see https://stackoverflow.com/a/27123302/441266).
   
   I moved `RFLAG_HAS_REMOVALS` from `MarkupContainer` to `Component` so we do not accidentally assign the same bit to two different flags.
   
   See https://issues.apache.org/jira/projects/WICKET/issues/WICKET-6839


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [wicket] theigl commented on pull request #459: WICKET-6839 Use request flag for caching `isVisibleInHierarchy`

GitBox

theigl commented on pull request #459:
URL: https://github.com/apache/wicket/pull/459#issuecomment-715234836


   The build fails because of Clirr:
   
   ```
   Error:  6004: org.apache.wicket.Component: Changed type of field RFLAG_CONTAINER_DEQUEING from short to int
   Warning:  6003: org.apache.wicket.Component: Value of compile-time constant RFLAG_CONTAINER_DEQUEING has been changed
   Error:  7005: org.apache.wicket.Component: Parameter 1 of 'protected boolean getRequestFlag(short)' has changed its type to int
   ```
   
   Neither the constant nor the method are part of the public API. Is there any way to tell Clirr to ignore these changes?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [wicket] svenmeier commented on pull request #459: WICKET-6839 Use request flag for caching `isVisibleInHierarchy`

GitBox
In reply to this post by GitBox

svenmeier commented on pull request #459:
URL: https://github.com/apache/wicket/pull/459#issuecomment-715290787


   Actually we still have one spare bit (short)0x8000 ... yes this is still a short :P
   
   I'd suggest you keep the change small:
   0x4 has been the value I've borrowed when adding RFLAG_ON_CONFIGURE_SUPER_CALL_VERIFIED, so it's time to reclaim it for the hierarchy:
   
   ```
   private static final short RFLAG_VISIBLE_IN_HIERARCHY_SET_VALUE = 0x4;
   private static final short RFLAG_ON_CONFIGURE_SUPER_CALL_VERIFIED = (short)0x8000;
   
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [wicket] theigl commented on pull request #459: WICKET-6839 Use request flag for caching `isVisibleInHierarchy`

GitBox
In reply to this post by GitBox

theigl commented on pull request #459:
URL: https://github.com/apache/wicket/pull/459#issuecomment-715306120


   @svenmeier: I wasn't aware of that! I just pushed the simplified change.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [wicket] theigl edited a comment on pull request #459: WICKET-6839 Use request flag for caching `isVisibleInHierarchy`

GitBox
In reply to this post by GitBox

theigl edited a comment on pull request #459:
URL: https://github.com/apache/wicket/pull/459#issuecomment-715306120


   @svenmeier: I wasn't aware of that! I just pushed the simplified change.
   
   I kept the more readable formatting for the flags, but I can revert that as well if you want the change to be smaller still.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [wicket] theigl merged pull request #459: WICKET-6839 Use request flag for caching `isVisibleInHierarchy`

GitBox
In reply to this post by GitBox

theigl merged pull request #459:
URL: https://github.com/apache/wicket/pull/459


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [wicket] theigl commented on pull request #459: WICKET-6839 Use request flag for caching `isVisibleInHierarchy`

GitBox
In reply to this post by GitBox

theigl commented on pull request #459:
URL: https://github.com/apache/wicket/pull/459#issuecomment-715351339


   Thank you @svenmeier and @martin-g!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]