aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stephan Erb <s...@apache.org>
Subject Re: Review Request 52921: In immutable Thrift structs, check identity before comparing fields.
Date Tue, 18 Oct 2016 20:13:36 GMT


> On Oct. 17, 2016, 5:41 p.m., Joshua Cohen wrote:
> > Should we also add:
> > 
> >     if (o == null) {
> >       return false;
> >     }
> > 
> > ?
> > 
> > Curious if we're able to see a difference in benchmarks from this change?
> 
> John Sirois wrote:
>     It would be a bit of black magic if that were significantly faster than the !instanceof
check.

I tried to run the benchmarks https://issues.apache.org/jira/browse/AURORA-1796

For those benchmarks that worked and finished in a reasonable timeframe, this RB did not show
any noticable improvement. I believe it is still safe to proceed and will therefore go ahead
and merge this as I got one ship-it.


- Stephan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52921/#review152889
-----------------------------------------------------------


On Oct. 17, 2016, 1:04 a.m., Stephan Erb wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52921/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2016, 1:04 a.m.)
> 
> 
> Review request for Aurora and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> I saw THRIFT-3868 and thought we could apply the same micro-optimization
> as well. Details: https://github.com/apache/thrift/pull/1106
> 
> Example of a generated equals method (in ITaskConfig):
> 
>     @Override
>     public boolean equals(Object o) {
>       if (this == o) {
>         return true;
>       }
>       if (!(o instanceof ITaskConfig)) {
>         return false;
>       }
>       ITaskConfig other = (ITaskConfig) o;
>       return Objects.equals(job, other.job)
>           && Objects.equals(owner, other.owner)
>           && Objects.equals(isService, other.isService)
>           && Objects.equals(numCpus, other.numCpus)
>           && Objects.equals(ramMb, other.ramMb)
>           && Objects.equals(diskMb, other.diskMb)
>           && Objects.equals(priority, other.priority)
>           && Objects.equals(maxTaskFailures, other.maxTaskFailures)
>           && Objects.equals(production, other.production)
>           && Objects.equals(tier, other.tier)
>           && Objects.equals(resources, other.resources)
>           && Objects.equals(constraints, other.constraints)
>           && Objects.equals(requestedPorts, other.requestedPorts)
>           && Objects.equals(mesosFetcherUris, other.mesosFetcherUris)
>           && Objects.equals(taskLinks, other.taskLinks)
>           && Objects.equals(contactEmail, other.contactEmail)
>           && Objects.equals(executorConfig, other.executorConfig)
>           && Objects.equals(metadata, other.metadata)
>           && Objects.equals(container, other.container);
>     }
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/tools/java/thrift_wrapper_codegen.py 7c281800b00b973351659ecacd5b7f6e55e46bba

> 
> Diff: https://reviews.apache.org/r/52921/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message