-->
These old forums are deprecated now and set to read-only. We are waiting for you on our new forums!
More modern, Discourse-based and with GitHub/Google/Twitter authentication built-in.

All times are UTC - 5 hours [ DST ]



Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 31 posts ]  Go to page Previous  1, 2, 3  Next
Author Message
 Post subject:
PostPosted: Mon Jun 02, 2008 6:50 pm 
Newbie

Joined: Sun Mar 30, 2008 12:19 am
Posts: 15
bodrin wrote:
We don't need this, because we already have the mapping :


Annotations are actually extremely expensive to read from in terms of code complexity. Annotations are also accessed (at least by the java 6 VM) by creating a Proxy object around the annotation as an interface, and then (I believe, I haven't examined the source) using JNI to read the information.

The amount of overhead of keeping that annotation object around would probably offset any space savings of not having the data stored in the constraint validator, and not keeping that annotation around will cause temporary objects and performance problems.


Top
 Profile  
 
 Post subject:
PostPosted: Tue Jun 03, 2008 5:44 pm 
Hibernate Team
Hibernate Team

Joined: Sun Sep 14, 2003 3:54 am
Posts: 7256
Location: Paris, France
dwaite wrote:
bodrin wrote:
We don't need this, because we already have the mapping :


Annotations are actually extremely expensive to read from in terms of code complexity. Annotations are also accessed (at least by the java 6 VM) by creating a Proxy object around the annotation as an interface, and then (I believe, I haven't examined the source) using JNI to read the information.

The amount of overhead of keeping that annotation object around would probably offset any space savings of not having the data stored in the constraint validator, and not keeping that annotation around will cause temporary objects and performance problems.


Actually you're a little bit too dramatic :)
The init is just as expensive as reading a class file and is done only once the first time for a given class. But the cache checking is done in a synchronized method.
An annotation is an interface so the object representing the annotation is a proxy impolementing the interface, it's not that bad though.

_________________
Emmanuel


Top
 Profile  
 
 Post subject:
PostPosted: Tue Jun 03, 2008 6:01 pm 
Hibernate Team
Hibernate Team

Joined: Fri Oct 05, 2007 4:47 pm
Posts: 2536
Location: Third rock from the Sun
Quote:
But the cache checking is done in a synchronized method.

If I may add to Emmanuel's post this additional argument:
A clever implementation may even choose to avoid this using some more advanced java.util.concurrent construct, there are many good candidates to choose from; it could even be possible to use ThreadLocals for best scalability (worst for memory).

But again, this are implementation details: I think we just need to be sure the specs don't enforce bottlenecks, and it is looking ok IMHO.

_________________
Sanne
http://in.relation.to/


Top
 Profile  
 
 Post subject:
PostPosted: Wed Jun 04, 2008 7:55 am 
Beginner
Beginner

Joined: Thu Apr 17, 2008 5:47 pm
Posts: 26
Quote:
You can be thread-safe and have state without requiring synchronization as state is prepared at init time.

Yes, but with the cost of additional Constraint instance.

I'm sorry that I have to repeat it again, but ..
Don't you see that in hibernate there are 16 validators and you actually need initialization in just one of them !?
This is a real case - validating ralational database constraints!
15:1 - this is very very rare and the model is not aware of it, so what I'm trying to explain is : we should improve it!

The current model assumes that we always need to initialize a Constraint. This is WRONG. And the bad consequence is that we must instantiate Constraint once per each annotation (wasting memory).
So, an improved model should assume that by default the Constraint doesn't need initialization, but it is possible if it desires (interface ExtendedConstraint).


Quote:
Annotations are actually extremely expensive to read from in terms of code complexity.


Actually in both cases we should read them and also as you can see in java.lang.Class - they are cached:
Code:
...
    // Annotations cache
    private transient Map<Class, Annotation> annotations;
    private transient Map<Class, Annotation> declaredAnnotations;



Quote:
the tuple of SimpleConstraint + Annotation


No matther if the validation methadata is obtained via xml or annotations after analyzing it I cache it inside the validator this way:
Code:
Map<String, Annotation[]> constraintDescriptorsMap

So that for each property I have a list of annotations and I can find the validator via a ConstraintRepository instance which is injected into the validator during creation.

String propertyName -> Annotation[] annotations
annotations[i] ---ConstraintRepository --> Constraint


Top
 Profile  
 
 Post subject:
PostPosted: Wed Jun 04, 2008 12:55 pm 
Hibernate Team
Hibernate Team

Joined: Fri Oct 05, 2007 4:47 pm
Posts: 2536
Location: Third rock from the Sun
Hi,
I think you are all having some good ideas about nice optimizations that could be implemented;
Still I don't see how the current single interface/single initialization would stop a clever implementation from using your ideas.
Yes of course reading annotations has some cost, but I would really not worry about the startup timings at this stage.

I also have some practical solutions:
A clever implementation could look at the Annotation options, and see if it already has created a validator with same parameters, so actually reusing the same instances for both no-args validators and similarly used "arg-validators": you save even more memory,
and keep the design simple.

IMHO it is far more interesting to standardize a way to know if a validator is ThreadSafe or not, shouldn't we introduce a marking interface?

"premature optimization is the root of all evil." (Knuth, Donald)

kind regards,

_________________
Sanne
http://in.relation.to/


Top
 Profile  
 
 Post subject:
PostPosted: Thu Jun 05, 2008 4:49 pm 
Hibernate Team
Hibernate Team

Joined: Sun Sep 14, 2003 3:54 am
Posts: 7256
Location: Paris, France
s.grinovero wrote:
Hi,
I think you are all having some good ideas about nice optimizations that could be implemented;
Still I don't see how the current single interface/single initialization would stop a clever implementation from using your ideas.
Yes of course reading annotations has some cost, but I would really not worry about the startup timings at this stage.

I also have some practical solutions:
A clever implementation could look at the Annotation options, and see if it already has created a validator with same parameters, so actually reusing the same instances for both no-args validators and similarly used "arg-validators": you save even more memory,
and keep the design simple.

IMHO it is far more interesting to standardize a way to know if a validator is ThreadSafe or not, shouldn't we introduce a marking interface?

"premature optimization is the root of all evil." (Knuth, Donald)

kind regards,


That was my thoughts as well, you can apply some of the optimizations under the hood without having to impact your public contract.

A constraint validator implementation must be threadsafe sanne, it can be accessed by multiple threads at the same time during concurrent validations.

_________________
Emmanuel


Top
 Profile  
 
 Post subject:
PostPosted: Fri Jun 06, 2008 7:47 am 
Beginner
Beginner

Joined: Thu Apr 17, 2008 5:47 pm
Posts: 26
This is not an imact - this is an improvement of the public contract :)

The current public contract does not give enough freedom for optimizations:

There is no way to reuse a constraint validator instance.

The idea with analyzing if the annotations have the same parameters in order to reuse the same constraint validator instance is far more complex then just refactoring the model a little bit, so that we have a non-initializable constraints by default.

Note that if a constraint validator is non-initializable a single instance can be used in order to validate all the corresponding annotations.


Top
 Profile  
 
 Post subject:
PostPosted: Fri Jun 06, 2008 12:45 pm 
Hibernate Team
Hibernate Team

Joined: Sun Sep 14, 2003 3:54 am
Posts: 7256
Location: Paris, France
It makes even more sense to reuse validators when they actually require a memory consuming initialization. In this situation the optimization leak to the API you propose will not help.

_________________
Emmanuel


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jun 07, 2008 1:57 pm 
Beginner
Beginner

Joined: Thu Apr 17, 2008 5:47 pm
Posts: 26
Quote:
It makes even more sense to reuse validators when they actually require a memory consuming initialization.


Having in mind that in 94% (1/16) of the cases you do not need any initialization at all it makes even more more sense to use a single validator instance.

Quote:
In this situation the optimization leak to the API you propose will not help.

In any situation the proposed API is better then the current one.

And for the validators there is no initialization that will not consume memory. You initialize in order to save some data (consume memory at least for the pointers + the new constraint instance) that will help you later when you validate.
If your initialization does not consume memory that means that you do not need initialization.


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jun 07, 2008 6:15 pm 
Hibernate Team
Hibernate Team

Joined: Fri Oct 05, 2007 4:47 pm
Posts: 2536
Location: Third rock from the Sun
Quote:
Having in mind that in 94% (1/16) of the cases you do not need any initialization at all it makes even more more sense to use a single validator instance.

Why should you just optimize 94% and not 100%?
As I said before you could detect the use of repeated options even on initializable validators, so getting and even higher reuse.
I'm sure it is easy to examinate the number and quality of parameters in implementations using the current API, with no slowdown at all after initialization.
So this ideas are good for a reference implementation (when the performance will have priority), still there is no need to complicate a clean API.

Quote:
A constraint validator implementation must be threadsafe sanne, it can be accessed by multiple threads at the same time during concurrent validations.

What I was thinking is that a no-initialize validator is probably always threadsafe and so wouldn't cause any problem (unless it uses static objects you should recommend against); but for validators needing an initialization the framework could help in making it threadsafe, actually wrapping the validator in threadlocals or similar. It would probably be safer and simpler for developers to have this requirement for threadsafety removed, hiding the complexity as EJB does; but also have the possibility to "mark" the validator as threadsafe for when the developer wants to do his own housekeeping.

IMHO the framework would trust the developers too much in requiring all implementations to be threadsafe; the worst part is that when they(we) forget it we probably won't detect the mistake in standard tests.

_________________
Sanne
http://in.relation.to/


Top
 Profile  
 
 Post subject:
PostPosted: Tue Jun 17, 2008 3:58 pm 
Beginner
Beginner

Joined: Thu Apr 17, 2008 5:47 pm
Posts: 26
Quote:
...
, still there is no need to complicate a clean API.


does clean api means: one interface is better then two interfaces? .. bacase i think the proposed api is cleaner then the current

Actually the approach whith analysing the parameters, so that you can reuse an initializable constraint validator instance does not give you 100% at all.
The simplest example is when you have a non-initializable constraint validator, but all the corresponding annotations are with different parameres. Then you will instantiate the constraint validator for all of them while you can use a single instance.

So, this does not work.

There is only one clean solution (==100%) - the model should provide information if the constraint validator is initializable and a way to use it appropriately.


Top
 Profile  
 
 Post subject:
PostPosted: Sun Aug 31, 2008 10:21 am 
Newbie

Joined: Sun Aug 31, 2008 9:05 am
Posts: 4
Hello, I'm so newbie in the Hiberante world, but i'm using it for some months ago, and really very happy with direction on JSR-303 that seem to standardize validation task.

I agree with 'bodrin' but not at all, my obserbation of the current problem, that in fact can be 3 scenarios for validator instance:

1) The validator not need initialization at all, and no receive parameters.
This can be the more common case that 'bodrin' point, and in fact most of the validation instances are working on this scenario:(AssertFalseValidator, AssertTrueValidator, EANValidator, FutureValidator, NotNullValidator, NotEmptyValidator, PastValidator)

2) The validator not need initialization at all, and receive parameters.
Here all that receive parameters and don't have any initialization process except copy required parameters for validation:
(DigitsValidator, LengthValidator, MaxValidator, MinValidator, RangeValidator , SizeValidator)

3) The validator need initialization with or without parameters.
With initialization, with or without parameters, the fact that not have parameters take a additional hint:
- In fact as i can see all annotations have a parameter 'message' that is not used at isValid() phase, so is hiberante validator create two different instances of validator impl? I need to check this I'm not sure.
(EmailValidator, PatternValidator)

1 - I also have a lot of validator implementations for our applications that fall back to this (is hiberntate treat the message parameter as different for two validators that have different messages?), so for this kind of implementation that in fact is thread safe, so only the Object value passed is used for validation.

2. - This case for fall to type 3 so validators that need parameters will run faster if it not need to access parameters via call (anotation.value()) that is proposed by 'borin' interface (boolean isValid(Object value, A constraintAnnotation)) i no check the generated byte code, but seems that access class property is more faster that method call, not sure at all, so I fall-back to type 3 so simplification of implementation, and no break current implementations.

3. - In this case we get some of our impls, also in case 2, but for me is the same case :), i think this case will implement void initialize(A constraintAnnotation); so in this case makes sense make a diferent instance for diferent parameters).

So for mu optimization point of view, there is a hint on implementation caching of validators to not duplicate instances when not needed, this can consider case 2 but if we revise case 3 seem that if a instance is created for different parameter will meet the specification, and no make duplicated instances, so my question is how different parameters for caching instances is treated?

I vote for this implementation and interfaces that will hit performance on initialization, use less memory at runtime for not initialized validators:

Code:
public interface Validator {
   /**
    * does the object/element pass the constraints
    */
   public boolean isValid(Object value);
}


Code:
public interface InitializedValidator<A extends Annotation> {
   /**
    * Take the annotations values
    *
    * @param parameters
    */
   public void initialize(A parameters);
}


This make the only implementation requirement are for all case 2 and case 3 are add the implements InitializedValidator<>, and for case 1 remove initialize decalrations.
Other change is in Caching validator instances, i not know the current implementation is done, so I revise source code and make some test cases, when I get some free time.

I apologize for simplify the API, i thick that two interfaces as saying 'bodrin' is best, so the validator implementation only depends on one simple method for simple validations that in fact is not using annotations at all, I think that this will fit for cases as pointed by 'borin' under memory constraints are hard, i'm fighting in the embedded world also :), so if this proposal of split interfaces are better focused on API implementation that on performance hit say me and split the issue in two:

1 - Performance issue in duplicate instances created fo the in fact same validator
2 - Simplify API implementation splitting the Validator interface in two more convenient interfaces Validator and InitializedValidator

Very impresive proyect, i'm really happy with standarized validation :).

Regards.

_________________
Marcos Lois Bermúdez


Last edited by securez on Sun Aug 31, 2008 12:23 pm, edited 1 time in total.

Top
 Profile  
 
 Post subject:
PostPosted: Sun Aug 31, 2008 10:36 am 
Newbie

Joined: Sun Aug 31, 2008 9:05 am
Posts: 4
Excuse my typpo i vote for this namming for JSR-303:

Validator -> Constraint
InitializedValidator -> InitializedConstraint

_________________
Marcos Lois Bermúdez


Top
 Profile  
 
 Post subject:
PostPosted: Sun Aug 31, 2008 4:09 pm 
Beginner
Beginner

Joined: Thu Apr 17, 2008 5:47 pm
Posts: 26
Hi, currently I have modified my interfaces a little bit:

Code:
public interface Constraint<A extends Annotation> {
   public void validate(ValidationContext<?> ctx);
}


and

Code:
public interface ExtendedConstraint<A extends Annotation> extends Constraint<A> {
   public void initialize(A constraintDescriptor);
}


You see that the validate method receives another type of parameter - the validation context. You can get the current bean, the current property (if not a bean level validation of course), the constrain descriptor and so on:

Code:
public interface ValidationContext<T> {

   public Class<T> getBeanClass();

   public Object getBean();

   public T getRootBean();

   public void setBean(Object bean);

   public void setBeanClass(Class<T> beanClass);

   public String getPropertyName();

   public void setPropertyName(String propertyName);

   public Object getPropertyValue();

   public void setPropertyValue(Object propertyValue);

   public Annotation getConstraintDescriptor();

   public void setConstraintDescriptor(Annotation constraintDescriptor);

   public ValidationListener getValidationListener();

   public ValidatorFactory getValidatorFactory();

   /**
    * Cascade validation to the current property if available.
    */
   public void cascade();

   /**
    * Cascade validation to the specified bean.
    *
    * @param bean - a new validation root bean
    */
   public void cascade(Object bean);

   public Stack<String> getPath();

   public boolean isValidated(Object bean);

   /**
    *
    * @param bean
    * @return false if the object was already validated, true - the object is added
    */
   public boolean addValidated(Object bean);

}


.. even more, you can also cascade validation on demend for the current property or jump directly to another object graph, check if some object has been already validated and so on..


Top
 Profile  
 
 Post subject:
PostPosted: Sun Aug 31, 2008 5:24 pm 
Newbie

Joined: Sun Aug 31, 2008 9:05 am
Posts: 4
So i think this interface implementation is not topic here, i think that this toipic is about performance, so for not confussing the people i continue this tread over interface implementation:

http://forum.hibernate.org/viewtopic.php?t=985340

I agree with you in split the interface in two, and i read some comments out there about the same think, i think that the right direction is keep the API simple as possible, exotic validation can even be performed at class level, so some of you requirements can be resolved with annotations (groups, conditionals, etc.), take a look a this JSR-303 pre implementation of a forum user:

http://code.google.com/p/agimatec-validation/

So making complex dependences on Constrain class implementation, i think will not the good direction.

Hope this helps, I really be happy if i see some draft about specification, but seems to be in early stages, so it's important that the spec will meet most cases as possible in a reasonable fashion.

Regards.

_________________
Marcos Lois Bermúdez


Top
 Profile  
 
Display posts from previous:  Sort by  
Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 31 posts ]  Go to page Previous  1, 2, 3  Next

All times are UTC - 5 hours [ DST ]


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum

Search for:
© Copyright 2014, Red Hat Inc. All rights reserved. JBoss and Hibernate are registered trademarks and servicemarks of Red Hat, Inc.