2016-12-30 4 views
1

RailsアプリケーションでXMLファイル用のフォルダを探し、解析してデータベースに保存するRakeタスクがあります。コードは正常に動作しますが、私は約2100ファイルの合計が1.5GBで、処理は非常に遅く、約400時間のファイルが7時間です。各XMLファイルには約600〜650の契約があり、各契約は0〜n個の添付ファイルを持つことができます。私はすべての値を貼り付けませんでしたが、各契約は25の値を持っています。遅いNokogiri解析を修正するには

私はActiverecordのImport gemを使用してプロセスを高速化するため、ファイルごとの配列とファイル全体が解析されるときに配列を作成しています。私はPostgresに大量インポートを行います。レコードが見つかった場合にのみ直接更新され、新しい添付ファイルが挿入されますが、これは100000レコードのうちの1つのレコードと同じです。これは、契約ごとに新しいレコードを作成するのではなく、ちょっとした助けになりますが、遅い部分がXML解析であることがわかりました。あなたが私の解析に何か間違っているのを見てください。

私が構築しているアレイを印刷しようとしたとき、遅い部分はファイル全体を読み込んだ/解析して配列で配列を印刷するまででした。 Nokogiriが起動する前に、NokogiriがXML全体を読み込んでいるので、速度を伴うprobemが解析中であると思われるのはなぜですか。

require 'nokogiri' 
require 'pp' 
require "activerecord-import/base" 

ActiveRecord::Import.require_adapter('postgresql') 
namespace :loadcrz2 do 
    desc "this task load contracts from crz xml files to DB" 
    task contracts: :environment do 
    actual_dir = File.dirname(__FILE__).to_s 
    Dir.foreach(actual_dir+'/../../crzfiles') do |xmlfile| 

     next if xmlfile == '.' or xmlfile == '..' or xmlfile == 'archive' 

     page = Nokogiri::XML(open(actual_dir+"/../../crzfiles/"+xmlfile)) 
     puts xmlfile 
     cons = page.xpath('//contracts/*') 
     contractsarr = [] 
     @c =[] 
     cons.each do |contract| 
      name = contract.xpath("name").text 
      crzid = contract.xpath("ID").text 
      procname = contract.xpath("procname").text 
      conname = contract.xpath("contractorname").text 
      subject = contract.xpath("subject").text 
      dateeff = contract.xpath("dateefficient").text 
      valuecontract = contract.xpath("value").text 

      attachments = contract.xpath('attachments/*') 
      attacharray = [] 
      attachments.each do |attachment| 
       attachid = attachment.xpath("ID").text 
       attachname = attachment.xpath("name").text 
       doc = attachment.xpath("document").text 
       size = attachment.xpath("size").text 

       arr = [attachid,attachname,doc,size] 
       attacharray.push arr 
      end 
      @con = Crzcontract.find_by_crzid(crzid) 
      if @con.nil? 
       @c=Crzcontract.new(:crzname => name,:crzid => crzid,:crzprocname=>procname,:crzconname=>conname,:crzsubject=>subject,:dateeff=>dateeff,:valuecontract=>valuecontract) 
      else 
       @con.crzname = name 
       @con.crzid = crzid 
       @con.crzprocname=procname 
       @con.crzconname=conname 
       @con.crzsubject=subject 
       @con.dateeff=dateeff 
       @con.valuecontract=valuecontract 
       @con.save! 
      end 
      attacharray.each do |attar| 
      attachid=attar[0] 
      attachname=attar[1] 
      doc=attar[2] 
      size=attar[3] 

       @at = Crzattachment.find_by_attachid(attachid) 
       if @at.nil? 

       if @con.nil? 
        @c.crzattachments.build(:attachid=>attachid,:attachname=>attachname,:doc=>doc,:size=>size) 
       else 
        @a=Crzattachment.new 
        @a.attachid = attachid 
        @a.attachname = attachname 
        @a.doc = doc 
        @a.size = size 
        @[email protected] 
        @a.save! 
       end 
       end 

     end 
      if @c.present? 
      contractsarr.push @c 
      end 
      #p @c 

     end 
     #p contractsarr 

     puts "done" 
     if contractsarr.present? 
     Crzcontract.import contractsarr, recursive: true 
     end 
     FileUtils.mv(actual_dir+"/../../crzfiles/"+xmlfile, actual_dir+"/../../crzfiles/archive/"+xmlfile) 


     end 
    end 
end 
+0

name = contract.xpath("name").text crzid = contract.xpath("ID").text procname = contract.xpath("procname").text 

あなたのような何かを行うことができます。このコードには多くの問題があります。 '@c = []'、 '@c = Crzcontract.new'、' if @ c.present? '(ArrayまたはCrzcontractインスタンスを意味しますか?大量のインポート。ノコギリがあなたの問題ではないかというと、論理的なデータベースのヒット数やロジックの数は、すべての情報が同じであれば、「更新する」というコンセプトが全く影響を及ぼしていない可能性があるからです。このロジックをクラスに分割して、何が起きているのかをよりよく判断できるようにします。 – engineersmnky

+0

ロジックは、契約が見つからない場合は@cを空として設定し、activerecord-importの新しい配列を作成してこの配列をcontractsarrにプッシュします私が書いたように@conロジックはすぐに保存するのに使われますが、100kあたり1レコードしかないので、これはこれは、私がactiverecord-import gemでupsertの問題を抱えていたのと同じように、ちょっとした回避策です。 –

+0

'@ con'が' nil'の場合、インスタンス変数 '@c'を上書きしています。 – engineersmnky

答えて

1

コードには多くの問題があります。ここではそれを改善するためのいくつかの方法があります:

actual_dir = File.dirname(__FILE__).to_s 

to_sを使用しないでください。 dirnameはすでに文字列を返しています。

actual_dir+'/../../crzfiles'、末尾のパスの区切り文字の有無は繰り返し使用されます。 Rubyに連結文字列を何度も再構築させないでください。代わりに、一度それを定義しますが、完全なパスを構築するRubyの能力を活用する:

File.absolute_path('../../bar', '/path/to/foo') # => "/path/bar" 

ので、使用します。その後、

actual_dir = File.absolute_path('../../crzfiles', __FILE__) 

のみactual_dirを参照してください。

Dir.foreach(actual_dir) 

これは、扱いにくい:

next if xmlfile == '.' or xmlfile == '..' or xmlfile == 'archive' 

私がやると思います。でも

next if (xmlfile[0] == '.' || xmlfile == 'archive') 

かを:

next if xmlfile[/^(?:\.|archive)/] 

は、これらの比較:

'.hidden'[/^(?:\.|archive)/] # => "." 
'.'[/^(?:\.|archive)/] # => "." 
'..'[/^(?:\.|archive)/] # => "." 
'archive'[/^(?:\.|archive)/] # => "archive" 
'notarchive'[/^(?:\.|archive)/] # => nil 
'foo.xml'[/^(?:\.|archive)/] # => nil 
それは '.'で始まるか 'archive'と等しい場合

パターンがtruthy値を返します。 。それは読みやすいものではなく、コンパクトです。私は化合物の条件付きテストをお勧めしたいと思います。いくつかの場所で

、あなたが xmlfileを連結しているので、再びRubyは一度それをやらせる:

xml_filepath = File.join(actual_dir、XMLFILE)どんなOSのファイルパスの区切り文字を尊重する

あなたは走っています。そして、代わりに名前を連結したxml_filepathを使用します。

xml_filepath = File.join(actual_dir, xmlfile))) 
page = Nokogiri::XML(open(xml_filepath)) 

[...] 

FileUtils.mv(xml_filepath, File.join(actual_dir, "archive", xmlfile) 

joinはので、それを活用する優れたツールです。文字列を連結するための別の名前ではなく、コードが実行されているOSで使用する正しい区切り文字も認識しているためです。

あなたがのインスタンスをたくさん使う

xpath("some_selector").text 

はそれをしないでください。 xpathcssおよびsearchと一緒にNodeSetを返し、NodeSetで使用するとtextは非常に急な滑りやすい斜面を邪魔するように悪いことがあります。このことを考えてみましょう:

require 'nokogiri' 

doc = Nokogiri::XML(<<EOT) 
<root> 
    <node> 
    <data>foo</data> 
    </node> 
    <node> 
    <data>bar</data> 
    </node> 
</root> 
EOT 

doc.search('//node/data').class # => Nokogiri::XML::NodeSet 
doc.search('//node/data').text # => "foobar" 

テキストの連結「をfoobarに」内には、簡単に分割することはできませんし、それは我々があまりにも頻繁に質問はこちらをご覧問題です。

あなたが理由searchxpathまたはcssを使用してのバックノードセットを取得予想される場合、これを行います。

doc.search('//node/data').map(&:text) # => ["foo", "bar"] 

それはあなたが特定のノード後にしているならば、text意志のでatat_xpathまたはat_cssを使用することをお勧めします期待どおりの仕事。

How to avoid joining all text from Nodes when scraping」も参照してください。

DRYされる可能性のある複製がたくさんあります。この代わりに:私は、これはあなたがこれをやっていると思う何をやっているとは思わない

name, crzid, procname = [ 
    'name', 'ID', 'procname' 
].map { |s| contract.at(s).text } 
関連する問題