> On Sept. 24, 2015, 6:57 a.m., Prasanth_J wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, 
> > line 5750
> > <https://reviews.apache.org/r/38702/diff/1/?file=1083680#file1083680line5750>
> >
> >     This is very hacky.
> >     I think reflection will be slower than ByteBuffer.put(). As put() uses 
> > intrinsic method (HeapBB) or unsafe copy (DirectBB) both should be faster 
> > than reflect. Can you get rid of reflection?

Buffer can be megabytes for some files... I don't think a Mb memory copy will 
be faster than reflection on an already-setAccessible field setter.


> On Sept. 24, 2015, 6:57 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java, line 1668
> > <https://reviews.apache.org/r/38702/diff/1/?file=1083686#file1083686line1668>
> >
> >     I am guessing the reason for getWithFastCheck is to avoid expensive 
> > compat check during split generation for single query case. We cannot cache 
> > hiveconf object across queries as configs may not be the same. If thats the 
> > case, why not store Hive object in a ThreadLocal?

There's a comment to that effect... because we'd need to clean all the 
threadlocals between queries. Also, yes, when conf is set for the new query, 
fast check will fail and the new one will be created.


> On Sept. 24, 2015, 6:57 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java, line 1533
> > <https://reviews.apache.org/r/38702/diff/1/?file=1083686#file1083686line1533>
> >
> >     Too many args for simple get/put pair.
> >     
> >     FileInfo[] getAll(Collection<HdfsFileStatusWithId> files);
> >     FileInfo get(HdfsFileStatusWithId file) (optional);
> >     
> >     void putAll(Collection<HdfsFileStatusWithId> files) (optional);
> >     void put(HdfsFileStatusWithId file, FileInfo, fileInfo);
> >     
> >     In fact, FooterCache interface can be generic.
> >     
> >     This will have all information for both cache implementation I guess. 
> > Right?

It will be a prettier interface but it will mean we'd need to open the reader 
again even though it's already open, etc.
This is an internal class for OIF... I don't think it's necessary to have a 
generic interface.


- Sergey


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


On Sept. 24, 2015, 1:03 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38702/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2015, 1:03 a.m.)
> 
> 
> Review request for hive and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java f3e2168 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 815f499 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 
> 6f15fd0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 
> e4a6cdb 
>   ql/pom.xml 36b3433 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 3511e73 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/HiveSplitGenerator.java 
> 87881b6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 2500fb6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcSplit.java cc03df7 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/ReaderImpl.java ab539c4 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 99896c6 
> 
> Diff: https://reviews.apache.org/r/38702/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Reply via email to